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

Merge api on Create or update file contents #29419

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

Conversation

smallfish06
Copy link

@smallfish06 smallfish06 commented Feb 26, 2024

This PR is related to (#19008).

After this PR merged, CreateFile API get deprecated and UpdateFile became CreateAndUpdateFile. Also some signature changed.

Added another PR on https://gitea.com/gitea/go-sdk/pulls/652, connected with this PR

I tried to make generate-swagger on my mac, but it keeps panicing even on main branch.. Is this expected behavior?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 26, 2024
@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 26, 2024
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Feb 26, 2024
Comment on lines +726 to +730
if apiOpts.SHA == "" {
ctx.JSON(http.StatusCreated, fileResponse)
} else {
ctx.JSON(http.StatusOK, fileResponse)
}
Copy link
Author

Choose a reason for hiding this comment

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

This part looks bit awkward to use SHA as flag of response. If you have any idea for this please let me know

I tried using file_service.GetContents to make clear for this but couldn't get more understandable code

@@ -41,7 +41,7 @@ func ApplyDiffPatch(ctx *context.APIContext) {
// in: body
// required: true
// schema:
// "$ref": "#/definitions/UpdateFileOptions"
// "$ref": "#/definitions/ApplyDiffPatchFileOptions"
Copy link
Author

Choose a reason for hiding this comment

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

Swagger doc was using wrong schema

@@ -50,95 +49,7 @@ func getCreateFileOptions() api.CreateFileOptions {
}
}

func getExpectedFileResponseForCreate(repoFullName, commitID, treePath, latestCommitSHA string) *api.FileResponse {
Copy link
Member

Choose a reason for hiding this comment

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

Why delete these functions and tests?

Copy link
Author

Choose a reason for hiding this comment

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

Duplicate code and testing. Same function and test are in api_repo_file_create_or_update_test.go

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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants