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

Generalize list header for API #16551

Merged
merged 65 commits into from
Aug 12, 2021
Merged

Conversation

6543
Copy link
Member

@6543 6543 commented Jul 26, 2021

close #11114

@6543 6543 added type/enhancement An improvement of existing functionality modifies/api This PR adds API routes or modifies them labels Jul 26, 2021
@6543 6543 added this to the 1.16.0 milestone Jul 26, 2021
@6543 6543 added the pr/wip This PR is not ready for review label Jul 26, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2021

Codecov Report

Merging #16551 (d499354) into main (ca13e1d) will increase coverage by 0.03%.
The diff coverage is 48.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16551      +/-   ##
==========================================
+ Coverage   45.36%   45.40%   +0.03%     
==========================================
  Files         758      758              
  Lines       85120    85258     +138     
==========================================
+ Hits        38612    38708      +96     
- Misses      40245    40277      +32     
- Partials     6263     6273      +10     
Impacted Files Coverage Δ
models/access.go 71.34% <0.00%> (ø)
models/org_team.go 55.19% <ø> (-0.01%) ⬇️
models/repo_collaboration.go 58.62% <0.00%> (-0.82%) ⬇️
models/repo_generate.go 15.00% <0.00%> (ø)
models/repo_transfer.go 43.47% <0.00%> (ø)
models/ssh_key.go 50.96% <0.00%> (-0.60%) ⬇️
models/token.go 66.66% <0.00%> (-5.75%) ⬇️
models/user.go 54.40% <0.00%> (-0.13%) ⬇️
modules/context/org.go 37.73% <0.00%> (ø)
routers/api/v1/admin/adopt.go 0.00% <0.00%> (ø)
... and 75 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca13e1d...d499354. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 26, 2021
CONTRIBUTING.md Outdated Show resolved Hide resolved
Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 28, 2021
@6543
Copy link
Member Author

6543 commented Jul 29, 2021

@zeripath your commit break this pull :/

zeripath and others added 2 commits July 29, 2021 20:22
Signed-off-by: Andrew Thornton <art27@cantab.net>
@6543
Copy link
Member Author

6543 commented Jul 29, 2021

@lunny anything else you like to know?

@KN4CK3R
Copy link
Member

KN4CK3R commented Aug 9, 2021

A helper method may be useful. At the moment there are 37 duplicated code blocks of

ctx.Header().Set("X-Total-Count", fmt.Sprintf("%d", totalCount))
ctx.Header().Set("Access-Control-Expose-Headers", "X-Total-Count")

If the "Access-Control-Expose-Headers" header is already present the method must append ", X-Total-Count" instead of replacing the old value.
Maybe SetLinkHeader could be extended.

@6543
Copy link
Member Author

6543 commented Aug 9, 2021

had that in mind to ... was lazy and thought ... do it next time 😅 - u catched me

will refactor bit more then ...

@6543 6543 requested a review from KN4CK3R August 10, 2021 00:00
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 12, 2021
@6543
Copy link
Member Author

6543 commented Aug 12, 2021

🚀

@6543 6543 merged commit 2289580 into go-gitea:main Aug 12, 2021
@6543 6543 deleted the api-generalize-list-header branch August 12, 2021 12:43
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
@zeripath zeripath changed the title [API] generalize list header Generalize list header for API Jan 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them type/enhancement An improvement of existing functionality type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Generalize Header for get XY list endpoints
6 participants