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

feat: add global variables for all query fields #12

Merged
merged 6 commits into from
Mar 4, 2025

Conversation

fnoopv
Copy link
Contributor

@fnoopv fnoopv commented Mar 3, 2025

As discussed in go-goyave/goyave#242, QueryParamPage and QueryParamPerPage are added to change the current page number field name and the data size per page field name in pagination parameters.

@System-Glitch System-Glitch self-requested a review March 3, 2025 15:59
@System-Glitch System-Glitch added the enhancement New feature or request label Mar 3, 2025
@System-Glitch System-Glitch self-assigned this Mar 3, 2025
@System-Glitch
Copy link
Member

While we're at it, can we do this for all the query parameters? (filter, or, search, sort etc) This would give maximum flexibility to the users of this library.

@fnoopv
Copy link
Contributor Author

fnoopv commented Mar 3, 2025

While we're at it, can we do this for all the query parameters? (filter, or, search, sort etc) This would give maximum flexibility to the users of this library.

No problem, I'll add them later

@fnoopv
Copy link
Contributor Author

fnoopv commented Mar 3, 2025

Should we add a FieldName field at the end of all fields to better express the function of the field? The current field name may be ambiguous, for example, QueryParamPerPage may be more like setting the size of PerPage, change to QueryParamPerPageFieldName may be better

@System-Glitch
Copy link
Member

Should we add a FieldName field at the end of all fields to better express the function of the field? The current field name may be ambiguous, for example, QueryParamPerPage may be more like setting the size of PerPage, change to QueryParamPerPageFieldName may be better

I'm fine with QueryParamPerPage.

@fnoopv fnoopv changed the title feat: add global varibles QueryParamPage and QueryParamPerPage feat: add global variables for all query fields Mar 4, 2025
Copy link
Member

@System-Glitch System-Glitch left a comment

Choose a reason for hiding this comment

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

I left a few recommendations for the documentation. Otherwise it looks good to me! Thank you very much! 👍🏻

@coveralls
Copy link

coveralls commented Mar 4, 2025

Pull Request Test Coverage Report for Build 13654836363

Details

  • 20 of 20 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.043%

Totals Coverage Status
Change from base Build 13284264438: 0.0%
Covered Lines: 931
Relevant Lines: 940

💛 - Coveralls

fnoopv and others added 3 commits March 4, 2025 19:59
Co-authored-by: SystemGlitch <[email protected]>
Co-authored-by: SystemGlitch <[email protected]>
Co-authored-by: SystemGlitch <[email protected]>
@System-Glitch System-Glitch self-requested a review March 4, 2025 13:27
@System-Glitch System-Glitch merged commit 51fad25 into go-goyave:master Mar 4, 2025
5 checks passed
@System-Glitch
Copy link
Member

Thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants