Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit18b178e

Browse files
lunnywxiaoguang
andauthored
Creating push comments before invoke pull request checking (#35647) (#35668)
Backport#35647 This PR moved the creation of pushing comments before pull requestmergeable checking. So that when the pull request status changed, thecomments should have been created.Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent1644b87 commit18b178e

File tree

4 files changed

+32
-14
lines changed

4 files changed

+32
-14
lines changed

‎services/issue/comments.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
user_model"code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/gitrepo"
1717
"code.gitea.io/gitea/modules/json"
18+
"code.gitea.io/gitea/modules/log"
1819
"code.gitea.io/gitea/modules/timeutil"
1920
git_service"code.gitea.io/gitea/services/git"
2021
notify_service"code.gitea.io/gitea/services/notify"
@@ -151,15 +152,15 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
151152
}
152153

153154
// LoadCommentPushCommits Load push commits
154-
funcLoadCommentPushCommits(ctx context.Context,c*issues_model.Comment)(errerror) {
155+
funcLoadCommentPushCommits(ctx context.Context,c*issues_model.Comment)error {
155156
ifc.Content==""||c.Commits!=nil||c.Type!=issues_model.CommentTypePullRequestPush {
156157
returnnil
157158
}
158159

159160
vardata issues_model.PushActionContent
160-
err=json.Unmarshal([]byte(c.Content),&data)
161-
iferr!=nil {
162-
returnerr
161+
iferr:=json.Unmarshal([]byte(c.Content),&data);err!=nil {
162+
log.Debug("Unmarshal: %v",err)// no need to show 500 error to end user when the JSON is broken
163+
returnnil
163164
}
164165

165166
c.IsForcePush=data.IsForcePush
@@ -168,9 +169,15 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
168169
iflen(data.CommitIDs)!=2 {
169170
returnnil
170171
}
171-
c.OldCommit=data.CommitIDs[0]
172-
c.NewCommit=data.CommitIDs[1]
172+
c.OldCommit,c.NewCommit=data.CommitIDs[0],data.CommitIDs[1]
173173
}else {
174+
iferr:=c.LoadIssue(ctx);err!=nil {
175+
returnerr
176+
}
177+
iferr:=c.Issue.LoadRepo(ctx);err!=nil {
178+
returnerr
179+
}
180+
174181
gitRepo,closer,err:=gitrepo.RepositoryFromContextOrOpen(ctx,c.Issue.Repo)
175182
iferr!=nil {
176183
returnerr
@@ -179,10 +186,11 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
179186

180187
c.Commits,err=git_service.ConvertFromGitCommit(ctx,gitRepo.GetCommitsFromIDs(data.CommitIDs),c.Issue.Repo)
181188
iferr!=nil {
182-
returnerr
189+
log.Debug("ConvertFromGitCommit: %v",err)// no need to show 500 error to end user when the commit does not exist
190+
}else {
191+
c.CommitsNum=int64(len(c.Commits))
183192
}
184-
c.CommitsNum=int64(len(c.Commits))
185193
}
186194

187-
returnerr
195+
returnnil
188196
}

‎services/pull/merge.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,11 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
248248
}
249249
deferreleaser()
250250
deferfunc() {
251+
// This is a duplicated call to AddTestPullRequestTask (it will also be called by the post-receive hook, via a push queue).
252+
// This call will do some operations (push to base repo, sync commit divergence, add PR conflict check queue task, etc)
253+
// immediately instead of waiting for the "push queue"'s task. The code is from https://github.com/go-gitea/gitea/pull/7082.
254+
// But it's really questionable whether it's worth to do it ahead without waiting for the "push queue" task to run.
255+
// TODO: DUPLICATE-PR-TASK: maybe can try to remove this in 1.26 to see if there is any issue.
251256
goAddTestPullRequestTask(TestPullRequestOptions{
252257
RepoID:pr.BaseRepo.ID,
253258
Doer:doer,

‎services/pull/pull.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,8 @@ type TestPullRequestOptions struct {
379379
funcAddTestPullRequestTask(optsTestPullRequestOptions) {
380380
log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests",opts.RepoID,opts.Branch)
381381
graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) {
382-
// There is no sensible way to shut this down ":-("
383-
// If you don't let it run all the way then you will lose data
384-
// TODO: graceful: AddTestPullRequestTask needs to become a queue!
385-
382+
// this function does a lot of operations to various models, if the process gets killed in the middle,
383+
// there is no way to recover at the moment. The best workaround is to let end user push again.
386384
// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR.
387385
prs,err:=issues_model.GetUnmergedPullRequestsByHeadInfo(ctx,opts.RepoID,opts.Branch)
388386
iferr!=nil {
@@ -401,11 +399,15 @@ func AddTestPullRequestTask(opts TestPullRequestOptions) {
401399
continue
402400
}
403401

404-
StartPullRequestCheckImmediately(ctx,pr)
402+
// create push comment before check pull request status,
403+
// then when the status is mergeable, the comment is already in database, to make testing easy and stable
405404
comment,err:=CreatePushPullComment(ctx,opts.Doer,pr,opts.OldCommitID,opts.NewCommitID,opts.IsForcePush)
406405
iferr==nil&&comment!=nil {
407406
notify_service.PullRequestPushCommits(ctx,opts.Doer,pr,comment)
408407
}
408+
// The caller can be in a goroutine or a "push queue", "conflict check" can be time-consuming,
409+
// and the concurrency should be limited, so the conflict check will be done in another queue
410+
StartPullRequestCheckImmediately(ctx,pr)
409411
}
410412

411413
ifopts.IsSync {

‎services/pull/update.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
5959
}
6060

6161
deferfunc() {
62+
// The code is from https://github.com/go-gitea/gitea/pull/9784,
63+
// it seems a simple copy-paste from https://github.com/go-gitea/gitea/pull/7082 without a real reason.
64+
// TODO: DUPLICATE-PR-TASK: search and see another TODO comment for more details
6265
goAddTestPullRequestTask(TestPullRequestOptions{
6366
RepoID:pr.BaseRepo.ID,
6467
Doer:doer,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp