+
Skip to content

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

Merged
merged 4 commits into from
Oct 22, 2020

Conversation

mik629
Copy link
Contributor

@mik629 mik629 commented Oct 14, 2020

  • 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

* 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
@mik629 mik629 marked this pull request as draft October 14, 2020 16:37
@mik629 mik629 force-pushed the fix-query-pagination branch from 9df842b to 0165c43 Compare October 15, 2020 12:01
@mik629 mik629 marked this pull request as ready for review October 15, 2020 12:18
Copy link
Member

@vincentlauvlwj vincentlauvlwj left a 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) {
Copy link
Member

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.

Copy link
Contributor Author

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
Copy link
Member

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?

Copy link
Contributor Author

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
@mik629 mik629 requested a review from vincentlauvlwj October 22, 2020 05:18
@vincentlauvlwj vincentlauvlwj changed the base branch from master to v3.3.x October 22, 2020 16:34
@vincentlauvlwj vincentlauvlwj merged commit 5b5c6b0 into kotlin-orm:v3.3.x Oct 22, 2020
@mik629
Copy link
Contributor Author

mik629 commented Oct 23, 2020

Could you please mark it with hacktoberfest-accepted label? 🙏

@vincentlauvlwj
Copy link
Member

OK

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

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载