Skip to content

Commit 2a57ea3

Browse files
committed
make ScheduleAutoMerge share the same "delete branch after merge" logic
1 parent 6998025 commit 2a57ea3

File tree

1 file changed

+20
-22
lines changed

1 file changed

+20
-22
lines changed

routers/api/v1/repo/pull.go

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"code.gitea.io/gitea/modules/gitrepo"
2626
"code.gitea.io/gitea/modules/graceful"
2727
"code.gitea.io/gitea/modules/log"
28-
"code.gitea.io/gitea/modules/optional"
2928
"code.gitea.io/gitea/modules/setting"
3029
api "code.gitea.io/gitea/modules/structs"
3130
"code.gitea.io/gitea/modules/timeutil"
@@ -990,8 +989,26 @@ func MergePullRequest(ctx *context.APIContext) {
990989
message += "\n\n" + form.MergeMessageField
991990
}
992991

992+
prUnit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests)
993+
if err != nil {
994+
ctx.APIErrorInternal(err)
995+
return
996+
}
997+
998+
// for agit flow, we should not delete the agit reference after merge
999+
// FIXME: old code has that comment above. Is that comment valid? What would go wrong if a agit branch is deleted after merge?
1000+
// * If a agit branch can be deleted after merge, then fix the comment and maybe other related code
1001+
// * If a agit branch should not be deleted, then we need to fix the logic and add more tests
1002+
deleteBranchAfterMerge := pr.Flow == issues_model.PullRequestFlowGithub
1003+
if form.DeleteBranchAfterMerge != nil {
1004+
// if the form field is defined, it takes precedence over the repo setting equivalent
1005+
deleteBranchAfterMerge = deleteBranchAfterMerge && *form.DeleteBranchAfterMerge
1006+
} else {
1007+
// otherwise, we look at the repo setting to make the determination
1008+
deleteBranchAfterMerge = deleteBranchAfterMerge && prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge
1009+
}
1010+
9931011
if form.MergeWhenChecksSucceed {
994-
deleteBranchAfterMerge := optional.FromPtr(form.DeleteBranchAfterMerge).ValueOrDefault(false)
9951012
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, deleteBranchAfterMerge)
9961013
if err != nil {
9971014
if pull_model.IsErrAlreadyScheduledToAutoMerge(err) {
@@ -1037,26 +1054,7 @@ func MergePullRequest(ctx *context.APIContext) {
10371054
}
10381055
log.Trace("Pull request merged: %d", pr.ID)
10391056

1040-
prUnit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests)
1041-
if err != nil {
1042-
ctx.APIErrorInternal(err)
1043-
return
1044-
}
1045-
1046-
// for agit flow, we should not delete the agit reference after merge
1047-
// FIXME: old code has that comment above. Is that comment valid? What would go wrong if a agit branch is deleted after merge?
1048-
// * If a agit branch can be deleted after merge, then fix the comment and maybe other related code
1049-
// * If a agit branch should not be deleted, then we need to fix the logic and add more tests
1050-
shouldDeleteBranch := pr.Flow == issues_model.PullRequestFlowGithub
1051-
if form.DeleteBranchAfterMerge != nil {
1052-
// if the form field is defined, it takes precedence over the repo setting equivalent
1053-
shouldDeleteBranch = shouldDeleteBranch && *form.DeleteBranchAfterMerge
1054-
} else {
1055-
// otherwise, we look at the repo setting to make the determination
1056-
shouldDeleteBranch = shouldDeleteBranch && prUnit.PullRequestsConfig().DefaultDeleteBranchAfterMerge
1057-
}
1058-
1059-
if shouldDeleteBranch {
1057+
if deleteBranchAfterMerge {
10601058
// check permission even it has been checked in repo_service.DeleteBranch so that we don't need to
10611059
// do RetargetChildrenOnMerge
10621060
if err := repo_service.CanDeleteBranch(ctx, pr.HeadRepo, pr.HeadBranch, ctx.Doer); err == nil {

0 commit comments

Comments
 (0)