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.

Sign up for GitHub

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

Jump to bottom

Return responseText instead of string in some functions #28836

Merged
merged 3 commits into from
Jan 19, 2024
Merged

Return responseText instead of string in some functions #28836

merged 3 commits into from
Jan 19, 2024

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Jan 18, 2024

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 18, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 18, 2024
@github-actions github-actions bot added the modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin label Jan 18, 2024
@yp05327 yp05327 requested a review from wxiaoguang January 18, 2024 02:20
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 18, 2024 •
edited
Loading

It doesn't compile pass lint (returning and using a private struct seem to violate the common standard), and I guess it's better to avoid exposing internal struct like *responseText.

Update: maybe it's fine to rename responseText to ResponseText, since it seems to be a stable struct and doesn't have other internal/special usages.

@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 Jan 18, 2024
@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 Jan 18, 2024
@KN4CK3R KN4CK3R added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 18, 2024
@lunny lunny merged commit b60a7c3 into go-gitea:main Jan 19, 2024
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Jan 19, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 19, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 19, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
0fa3966 
* giteaofficial/main:
  Return `responseText` instead of string in some functions (go-gitea#28836)
  Fix display latest sync time for pull mirrors on the repo page (go-gitea#28841)
  Add testing for CalcCommitStatus (go-gitea#28823)
  Remove duplicated checkinit on git module (go-gitea#28824)
  Add missing migration (go-gitea#28827)
  Fix uploaded artifacts should be overwritten (go-gitea#28726)
henrygoodman pushed a commit to henrygoodman/gitea that referenced this pull request Jan 31, 2024
@yp05327
Return responseText instead of string in some functions (go-gitea#2…
34cb11f 
…8836)

Follow
go-gitea#28796 (comment)
@yp05327 yp05327 deleted the improve-call-requestJSONResp branch February 1, 2024 00:05
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
@yp05327 @silverwind
Return responseText instead of string in some functions (go-gitea#2…
66870e2 
…8836)

Follow
go-gitea#28796 (comment)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KN4CK3R KN4CK3R KN4CK3R approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.
6 participants
@yp05327 @wxiaoguang @KN4CK3R @delvh @lunny @GiteaBot