Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refine parameter and field names to align with PageRequest property names #2882

Closed

Conversation

thachlp
Copy link
Contributor

@thachlp thachlp commented Jul 18, 2023

We mix using page with pageNumber, size with pageSize and sometimes, it makes me confused. And I think pageNumber and pageSize are more clearer. Feel free to close this if there is a reason or we can not change it.

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

We mix using page with pageNumber, size with pageSize and sometimes, it makes me confused. And I think pageNumber and pageSize are more clearer.
Feel free to close this if there is a reason or we can not change it.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 18, 2023
@mp911de mp911de self-assigned this Jul 19, 2023
*/
public AbstractPageRequest(int page, int size) {
protected AbstractPageRequest(int pageNumber, int pageSize) {
Copy link
Member

Choose a reason for hiding this comment

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

We should not change visibility. I'm going to revert this change upon merge.

@@ -36,13 +37,13 @@ public class PageRequest extends AbstractPageRequest {
/**
* Creates a new {@link PageRequest} with sort parameters applied.
*
* @param page zero-based page index, must not be negative.
* @param size the size of the page to be returned, must be greater than 0.
* @param pageNumber zero-based pageNumber index, must not be negative.
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, seems too much was changed by the refactoring. I'll align these upon merge.

@mp911de mp911de added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 20, 2023
@mp911de mp911de changed the title Improve consistence parameters Refine parameter and field names to align with PageRequest property names Jul 20, 2023
@mp911de mp911de added this to the 3.0.9 (2022.0.9) milestone Jul 20, 2023
@mp911de mp911de closed this in 8ad79b2 Jul 20, 2023
mp911de added a commit that referenced this pull request Jul 20, 2023
Tweak Javadoc wording, revert visibility changes, add missing Override annotations.

See #2882
mp911de pushed a commit that referenced this pull request Jul 20, 2023
… names.

Align parameter naming with properties to not mix using page with pageNumber, size with pageSize names.

Closes #2882
mp911de added a commit that referenced this pull request Jul 20, 2023
Tweak Javadoc wording, revert visibility changes, add missing Override annotations.

See #2882
mp911de pushed a commit that referenced this pull request Jul 20, 2023
… names.

Align parameter naming with properties to not mix using page with pageNumber, size with pageSize names.

Closes #2882
mp911de added a commit that referenced this pull request Jul 20, 2023
Tweak Javadoc wording, revert visibility changes, add missing Override annotations.

See #2882
@mp911de
Copy link
Member

mp911de commented Jul 20, 2023

Thank you for your contribution. That's merged, polished, and backported now.

@jeffryang24
Copy link

jeffryang24 commented Feb 1, 2024

Hi @mp911de, I think we need to update the release note for Spring Data 2022.0 at https://github.com/spring-projects/spring-data-commons/wiki/Spring-Data-2022.0-(Turing)-Release-Notes, somehow I did not find it there when I was upgrading my service to Spring Boot 3.

I think this commit should be considered as breaking change since it may affect many user especially for a downstream service. Wdyt? 🙇

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants