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

Fix panic for fixBrokenRepoUnits16961 #30068

Merged
merged 5 commits into from
Mar 26, 2024
Merged

Fix panic for fixBrokenRepoUnits16961 #30068

merged 5 commits into from
Mar 26, 2024

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Mar 25, 2024

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 25, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 25, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 25, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Mar 25, 2024

Does doctor commands also have tests?

@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 Mar 25, 2024
@lunny
Copy link
Member

lunny commented Mar 25, 2024

Does doctor commands also have tests?

Should this be backport?

@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 Mar 25, 2024
@lunny lunny added type/bug reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. backport/v1.21 This PR should be backported to Gitea 1.21 labels Mar 25, 2024
wxiaoguang
wxiaoguang reviewed Mar 25, 2024
View reviewed changes
services/doctor/fix16961.go Outdated
@@ -249,6 +249,8 @@ func fixBrokenRepoUnit16961(repoUnit *repo_model.RepoUnit, bs []byte) (fixed boo
if fixed, err := fixIssuesConfig16961(bs, cfg); !fixed {
return false, err
}
case unit.TypeActions:
// action unit is newly added, so skip here
Copy link
Contributor

@wxiaoguang wxiaoguang Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe next time the panic would be caused by "Packages"

image

Copy link
Contributor

@wxiaoguang wxiaoguang Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I do not think the logic still make sense nowadays.

At least, it could first try to decode the JSON, if the decoding fails, then switch repoUnit.Type {} and fix, and keep the old logic.

Because new units shouldn't/couldn't break, and never need to be fixed.

@yp05327 yp05327 removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 25, 2024
@yp05327 yp05327 requested review from lunny and delvh March 26, 2024 01:34
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Mar 26, 2024
@yp05327 yp05327 requested a review from wxiaoguang March 26, 2024 01:34
@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 26, 2024
@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 Mar 26, 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 Mar 26, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 26, 2024
@lunny lunny merged commit 08aec2c into go-gitea:main Mar 26, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 26, 2024
@GiteaBot
Copy link
Contributor

I was unable to create a backport for 1.21. @yp05327, please send one manually. 🍵

go run ./contrib/backport 30068
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 26, 2024
@lunny lunny modified the milestones: 1.23.0, 1.22.0 Mar 26, 2024
@yp05327 yp05327 deleted the fix-fix16961 branch March 26, 2024 07:16
@yp05327 yp05327 mentioned this pull request Mar 26, 2024
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Mar 26, 2024
lunny pushed a commit that referenced this pull request Mar 26, 2024
@yp05327
Fix panic for fixBrokenRepoUnits16961 (#30068) (#30100)
6d47b63 
backport #30068
silverwind added a commit to silverwind/gitea that referenced this pull request Mar 26, 2024
@silverwind
Merge remote-tracking branch 'origin/main' into coloris
c36d353 
* origin/main:
  Remove jQuery `.attr` from the Fomantic modal cancel buttons (go-gitea#30113)
  Remove jQuery `.attr` from the code comments (go-gitea#30112)
  Remove jQuery calls that have no effect on `showElem` and `hideElem` (go-gitea#30110)
  Remove jQuery `.attr` from the common issue page functions (go-gitea#30083)
  Restore aligned grid column CSS (go-gitea#30106)
  Fix possible data race on tests (go-gitea#30093)
  Add svg linter and fix incorrect svgs (go-gitea#30086)
  Fix duplicate migrated milestones (go-gitea#30102)
  Update JS any PY dependencies, remove workarounds (go-gitea#30085)
  Fix gitea doctor will remove repo-avatar files when execute command `storage-archives` (go-gitea#30094)
  Fix alignment in actions right view (go-gitea#29979)
  Remove repetitive words (go-gitea#30091)
  Fix table header text-align (go-gitea#30084)
  Fix panic for `fixBrokenRepoUnits16961` (go-gitea#30068)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@lunny lunny lunny approved these changes

@delvh delvh Awaiting requested review from delvh

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.22.0
Development

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