Fix automerge will not work because of some events haven't been triggered #30780
+344
−63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lunny
added
type/bug
backport/v1.21
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/L
lunny
commented
May 7, 2024
|
I think we need some tests. |
wolfogre
approved these changes
May 21, 2024
GiteaBot
added
lgtm/need 1
wxiaoguang
previously approved these changes
May 21, 2024
GiteaBot
added
lgtm/done
wxiaoguang
reviewed
May 21, 2024
| @@ -111,6 +104,33 @@ func MergeScheduledPullRequest(ctx context.Context, sha string, repo *repo_model | |||
| return nil | |||
| } | |||
|
|
|||
| // StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request | |||
| func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) { | |||
There was a problem hiding this comment.
Why not make StartPRCheckAndAutoMergeBySHA call StartPRCheckAndAutoMerge to avoid duplicate code?
There was a problem hiding this comment.
They are two different functions. I think less code are duplicated.
There was a problem hiding this comment.
Actually I don't see why sha should be passed into addToQueue
It seems that the sha is only used for logging. Ideally only call addToQueue(pr.ID) should be enough, and a lot of code could be simplified?
There was a problem hiding this comment.
I think the SHA will prevent when both an automerge task and a newly push are triggered. So the previous triggered automerge task will be ignored.
if headCommitID != sha {
log.Warn("Head commit id of auto merge %-v does not match sha [%s], it may means the head branch has been updated. Just ignore this request because a new request expected in the queue", pr, sha)
return
}
There was a problem hiding this comment.
Actually such check doesn't seem right and only makes some problem harder to debug.
Because, the head commit ID could change again after that check.
There was a problem hiding this comment.
Yes, it's not perfect. It needs more refactor like I said before. But since it's a bugfix, I will not do it in this PR.
GiteaBot
added
lgtm/need 1
wxiaoguang
approved these changes
May 21, 2024
GiteaBot
added
lgtm/done
wxiaoguang
reviewed
May 21, 2024
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
GiteaBot
pushed a commit
to GiteaBot/gitea
that referenced
this pull request
May 21, 2024
…ered (go-gitea#30780) Replace go-gitea#25741 Close go-gitea#24445 Close go-gitea#30658 Close go-gitea#20646 ~Depends on go-gitea#30805~ Since go-gitea#25741 has been rewritten totally, to make the contribution easier, I will continue the work in this PR. Thanks @6543 --------- Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
lunny
added a commit
that referenced
this pull request
May 22, 2024
…ered (#30780) (#31039) Backport #30780 by @lunny Replace #25741 Close #24445 Close #30658 Close #20646 ~Depends on #30805~ Since #25741 has been rewritten totally, to make the contribution easier, I will continue the work in this PR. Thanks @6543 Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
zjjhot
added a commit
to zjjhot/gitea
that referenced
this pull request
May 22, 2024
* giteaofficial/main: Add nix flake for dev shell (go-gitea#30967) [skip ci] Updated translations via Crowdin Fix wrong display of recently pushed notification (go-gitea#25812) use existing oauth grant for public client (go-gitea#31015) Fix automerge will not work because of some events haven't been triggered (go-gitea#30780)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace #25741
Close #24445
Close #30658
Close #20646
Depends on #30805Since #25741 has been rewritten totally, to make the contribution easier, I will continue the work in this PR. Thanks @6543