Skip to content

Commit 6f2dfd7

Browse files
mislavmislav
committed
Adjust conditions for switching between regular and auto merge
Conditions prohibiting a regular merge: BLOCKED, BEHIND, DIRTY. Conditions triggering a regular merge even if `--auto` was set: CLEAN, HAS_HOOKS. Note that UNKNOWN status does not trigger either of the conditions.
1 parent 0ab9c70 commit 6f2dfd7

File tree

2 files changed

+60
-16
lines changed

2 files changed

+60
-16
lines changed

‎pkg/cmd/pr/merge/merge.go‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func mergeRun(opts *MergeOptions) error {
156156

157157
findOptions := shared.FindOptions{
158158
Selector: opts.SelectorArg,
159-
Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeable", "mergeStateStatus", "headRepositoryOwner", "headRefName"},
159+
Fields: []string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"},
160160
}
161161
pr, baseRepo, err := opts.Finder.Find(findOptions)
162162
if err != nil {
@@ -191,25 +191,17 @@ func mergeRun(opts *MergeOptions) error {
191191
}
192192
}
193193

194-
if pr.Mergeable == "CONFLICTING" {
195-
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) has conflicts and isn't mergeable\n", cs.Red("!"), pr.Number, pr.Title)
196-
return cmdutil.SilentError
197-
}
198-
199-
approved := pr.MergeStateStatus == "CLEAN" || pr.MergeStateStatus == "HAS_HOOKS" || pr.MergeStateStatus == "UNSTABLE"
200-
if !opts.AutoMergeEnable && !approved {
201-
fmt.Fprintf(
202-
opts.IO.ErrOut,
203-
"%s Merging is blocked. Enable auto-merge to automatically merge it when all requirements are met with: gh pr merge %d --auto\n",
204-
cs.FailureIcon(), pr.Number)
194+
isPRAlreadyMerged := pr.State == "MERGED"
195+
if blocked := blockedReason(pr.MergeStateStatus); !opts.AutoMergeEnable && !isPRAlreadyMerged && blocked != "" {
196+
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is not mergeable: %s.\n", cs.FailureIcon(), pr.Number, blocked)
197+
fmt.Fprintf(opts.IO.ErrOut, "To have the pull request merged after all the requirements have been met, add the `--auto` flag.\n")
205198
return cmdutil.SilentError
206199
}
207200

208201
deleteBranch := opts.DeleteBranch
209202
crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner()
210-
autoMerge := opts.AutoMergeEnable && !approved
203+
autoMerge := opts.AutoMergeEnable && !isImmediatelyMergeable(pr.MergeStateStatus)
211204

212-
isPRAlreadyMerged := pr.State == "MERGED"
213205
if !isPRAlreadyMerged {
214206
payload := mergePayload{
215207
repo: baseRepo,
@@ -461,3 +453,26 @@ func (e *userEditor) Edit(filename, startingText string) (string, error) {
461453

462454
return surveyext.Edit(editorCommand, filename, startingText, e.io.In, e.io.Out, e.io.ErrOut, nil)
463455
}
456+
457+
// blockedReason translates various MergeStateStatus GraphQL values into human-readable reason
458+
func blockedReason(status string) string {
459+
switch status {
460+
case "BLOCKED":
461+
return "the base branch policy prohibits the merge"
462+
case "BEHIND":
463+
return "the head branch is not up to date with the base branch"
464+
case "DIRTY":
465+
return "the merge commit cannot be cleanly created"
466+
default:
467+
return ""
468+
}
469+
}
470+
471+
func isImmediatelyMergeable(status string) bool {
472+
switch status {
473+
case "CLEAN", "HAS_HOOKS":
474+
return true
475+
default:
476+
return false
477+
}
478+
}

‎pkg/cmd/pr/merge/merge_test.go‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,35 @@ func TestPrMerge(t *testing.T) {
289289
}
290290
}
291291

292+
func TestPrMerge_blocked(t *testing.T) {
293+
http := initFakeHTTP()
294+
defer http.Verify(t)
295+
296+
shared.RunCommandFinder(
297+
"1",
298+
&api.PullRequest{
299+
ID: "THE-ID",
300+
Number: 1,
301+
State: "OPEN",
302+
Title: "The title of the PR",
303+
MergeStateStatus: "BLOCKED",
304+
},
305+
baseRepo("OWNER", "REPO", "master"),
306+
)
307+
308+
_, cmdTeardown := run.Stub()
309+
defer cmdTeardown(t)
310+
311+
output, err := runCommand(http, "master", true, "pr merge 1 --merge")
312+
assert.EqualError(t, err, "SilentError")
313+
314+
assert.Equal(t, "", output.String())
315+
assert.Equal(t, heredoc.Docf(`
316+
X Pull request #1 is not mergeable: the base branch policy prohibits the merge.
317+
To have the pull request merged after all the requirements have been met, add the %[1]s--auto%[1]s flag.
318+
`, "`"), output.Stderr())
319+
}
320+
292321
func TestPrMerge_nontty(t *testing.T) {
293322
http := initFakeHTTP()
294323
defer http.Verify(t)
@@ -471,7 +500,7 @@ func Test_nonDivergingPullRequest(t *testing.T) {
471500
stubCommit(pr, "COMMITSHA1")
472501

473502
prFinder := shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "master"))
474-
prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeable", "mergeStateStatus", "headRepositoryOwner", "headRefName"})
503+
prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"})
475504

476505
http.Register(
477506
httpmock.GraphQL(`mutation PullRequestMerge\b`),
@@ -510,7 +539,7 @@ func Test_divergingPullRequestWarning(t *testing.T) {
510539
stubCommit(pr, "COMMITSHA1")
511540

512541
prFinder := shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "master"))
513-
prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeable", "mergeStateStatus", "headRepositoryOwner", "headRefName"})
542+
prFinder.ExpectFields([]string{"id", "number", "state", "title", "lastCommit", "mergeStateStatus", "headRepositoryOwner", "headRefName"})
514543

515544
http.Register(
516545
httpmock.GraphQL(`mutation PullRequestMerge\b`),

0 commit comments

Comments
 (0)