-
-
Notifications
You must be signed in to change notification settings - Fork 154
Allow offset without limit #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow offset without limit #198
Conversation
* let the `offset` be specified without a `limit` * let the `limit` be specified without an `offset` * let `offset` and `limit` be specified together via separate routines
* prettify general `limit` routine * improve test coverage
9df842b
to
0165c43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! I will merge your code in the next version. Please also update the build.gradle
file, add your GitHub ID to the developers info (line 90).
is SelectExpression -> expression.copy(offset = offset, limit = limit) | ||
is UnionExpression -> expression.copy(offset = offset, limit = limit) | ||
} | ||
when (expression) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd prefer 4-space indent here, but there are 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const val ID_1 = 1 | ||
const val ID_2 = 2 | ||
const val ID_3 = 3 | ||
const val ID_4 = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have to define these numbers as constants. It makes no sense. Seems the quality check doesn't ignore the test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it doesn't
* add my name to contributors list * fix 8-space indents
Could you please mark it with |
OK |
offset
be specified without alimit
limit
be specified without anoffset
offset
andlimit
be specified together via separate routines