Skip to content
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

PageRequest.page(num) method can cause wrong usage #679

Conversation

njr-11
Copy link
Contributor

@njr-11 njr-11 commented Apr 8, 2024

fixes #672

This adds onto the change to remove next()/previous() (first commit) and also removes page(number) from PageRequest (second commit). All of these methods can mislead applications into writing code that causes wrong behavior.

@njr-11 njr-11 added this to the Jakarta Data 1.0 milestone Apr 8, 2024
@otaviojava
Copy link
Contributor

As I said, I like this as the first step, but it only fixes some problems.

The specialization will solve it entirely, but I know it has an impact.

@njr-11 njr-11 force-pushed the 672-pagerequest-page-method-can-cause-wrong-usage branch from c36e83c to 26ffc6f Compare April 9, 2024 13:35
@njr-11
Copy link
Contributor Author

njr-11 commented Apr 9, 2024

As I said, I like this as the first step, but it only fixes some problems.

The specialization will solve it entirely, but I know it has an impact.

It is not clear to me what problem the specialization solves. I asked Gavin for more clarification under #672 and came up with this to address what he identified.

@otaviojava
Copy link
Contributor

In term of impact, I prefer this one

@otaviojava otaviojava requested a review from gavinking April 9, 2024 17:32
@gavinking
Copy link
Contributor

OK, this looks much better.

@otaviojava otaviojava merged commit 85fa077 into jakartaee:main Apr 10, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: PageRequest doesn't know whether it's a Builder
3 participants