Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Order By in GroupModel select #26

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

pmvcoelho
Copy link

@pmvcoelho pmvcoelho commented Apr 7, 2019

Order By in union model select

@pmvcoelho pmvcoelho closed this Apr 7, 2019
@pmvcoelho pmvcoelho reopened this Apr 7, 2019
@pmvcoelho pmvcoelho changed the title Add files via upload Order By in UnionModel select Apr 7, 2019
@DarkSide666 DarkSide666 self-requested a review April 8, 2019 14:09
@DarkSide666 DarkSide666 changed the title Order By in UnionModel select Order By in GroupModel select Apr 8, 2019
Copy link
Member

@DarkSide666 DarkSide666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically looks good, but need to add some tests to validate that it works in all cases.

@romaninsh
Copy link
Member

romaninsh commented Aug 22, 2019

Thanks for your contribution @pmvcoelho!

This drops "setOrder" method, is that OK, Imatns? Would that be a BC break ?

also - I'd like to see why this change is needed and what are some use-cases. Preferable in documentation format but if just in the description of this pull request, it would be awesome!

@DarkSide666
Copy link
Member

#31 should go in first and then see what good we can take from this PR in.

@DarkSide666
Copy link
Member

@pmvcoelho now please add some tests which prove that your changes work fine.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants