Skip to content

fix: return 201 Created for CreateVariable API responses #34517

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

appleboy
Copy link
Member

@appleboy appleboy commented May 22, 2025

  • Change CreateVariable API response status from 204 No Content to 201 Created
  • Update related integration tests to expect 201 Created instead of 204 No Content

API SDK: https://gitea.com/gitea/go-sdk/pulls/713

- Change CreateVariable API response status from 204 No Content to 201 Created
- Update related integration tests to expect 201 Created instead of 204 No Content

Signed-off-by: Bo-Yi Wu <[email protected]>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 22, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels May 22, 2025
appleboy added 2 commits May 22, 2025 09:24
- Change API response status for variable creation from 204 No Content to 201 Created
- Update integration tests to expect 201 Created status after creating variables

Signed-off-by: Bo-Yi Wu <[email protected]>
- Change response status code from 204 No Content to 201 Created when creating a variable

Signed-off-by: Bo-Yi Wu <[email protected]>
@lunny
Copy link
Member

lunny commented May 22, 2025

swagger documentation need to be updated.

@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label May 22, 2025
@lunny lunny added this to the 1.25.0 milestone May 22, 2025
- Update variable creation responses to use repo-level terminology instead of org-level or generic descriptions
- Replace 404 not found responses with 409 conflict responses when a variable name already exists
- Remove redundant or outdated 204 response descriptions for variable creation endpoints

Signed-off-by: Bo-Yi Wu <[email protected]>
@appleboy
Copy link
Member Author

@lunny Done in 104b736

@silverwind
Copy link
Member

silverwind commented May 22, 2025

Changing 2xx to 2xx and 4xx to 4xx is probably a low-impact change, but could you add a note regarding the 4xx change to OP message and some textual justifications? Also please follow template from https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#how-to-handle-breaking-prs.

@lunny
Copy link
Member

lunny commented May 22, 2025

LGTM except #34517 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants