From 577d42280c692685884a112819b40d9c8a01f922 Mon Sep 17 00:00:00 2001 From: Ariel Deitcher <1149246+mntlty@users.noreply.github.com> Date: Mon, 3 Oct 2022 02:32:48 -0700 Subject: [PATCH] surface merge conflicts in pr status (#5999) --- .../pr/status/fixtures/prStatusChecks.json | 31 ++++++++++++++++++- pkg/cmd/pr/status/http.go | 17 ++++++---- pkg/cmd/pr/status/status.go | 21 +++++++++++-- pkg/cmd/pr/status/status_test.go | 5 +-- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/pr/status/fixtures/prStatusChecks.json b/pkg/cmd/pr/status/fixtures/prStatusChecks.json index 32ed236a9..91455ee2c 100644 --- a/pkg/cmd/pr/status/fixtures/prStatusChecks.json +++ b/pkg/cmd/pr/status/fixtures/prStatusChecks.json @@ -7,7 +7,7 @@ } }, "viewerCreated": { - "totalCount": 3, + "totalCount": 4, "edges": [ { "node": { @@ -16,6 +16,7 @@ "state": "OPEN", "url": "https://github.com/cli/cli/pull/8", "headRefName": "strawberries", + "mergeable": "UNKNOWN", "reviewDecision": "CHANGES_REQUESTED", "statusCheckRollup": { "nodes": [ @@ -98,6 +99,7 @@ "state": "OPEN", "url": "https://github.com/cli/cli/pull/6", "headRefName": "avo", + "mergeable": "MERGEABLE", "reviewDecision": "REVIEW_REQUIRED", "statusCheckRollup": { "nodes": [ @@ -125,6 +127,33 @@ ] } } + }, + { + "node": { + "number": 5, + "title": "Why can't berries get along?", + "state": "OPEN", + "url": "https://github.com/cli/cli/pull/5", + "headRefName": "strawberries", + "mergeable": "CONFLICTING", + "statusCheckRollup": { + "nodes": [ + { + "commit": { + "statusCheckRollup": { + "contexts": { + "nodes": [ + { + "state": "SUCCESS" + } + ] + } + } + } + } + ] + } + } } ] }, diff --git a/pkg/cmd/pr/status/http.go b/pkg/cmd/pr/status/http.go index 04dfebdcf..e0b2145e9 100644 --- a/pkg/cmd/pr/status/http.go +++ b/pkg/cmd/pr/status/http.go @@ -12,10 +12,11 @@ import ( ) type requestOptions struct { - CurrentPR int - HeadRef string - Username string - Fields []string + CurrentPR int + HeadRef string + Username string + Fields []string + ConflictStatus bool } type pullRequestsPayload struct { @@ -56,7 +57,7 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r fragments = fmt.Sprintf("fragment pr on PullRequest{%s}fragment prWithReviews on PullRequest{...pr}", gr) } else { var err error - fragments, err = pullRequestFragment(httpClient, repo.RepoHost()) + fragments, err = pullRequestFragment(repo.RepoHost(), options.ConflictStatus) if err != nil { return nil, err } @@ -186,12 +187,16 @@ func pullRequestStatus(httpClient *http.Client, repo ghrepo.Interface, options r return &payload, nil } -func pullRequestFragment(httpClient *http.Client, hostname string) (string, error) { +func pullRequestFragment(hostname string, conflictStatus bool) (string, error) { fields := []string{ "number", "title", "state", "url", "isDraft", "isCrossRepository", "headRefName", "headRepositoryOwner", "mergeStateStatus", "statusCheckRollup", "requiresStrictStatusChecks", } + + if conflictStatus { + fields = append(fields, "mergeable") + } reviewFields := []string{"reviewDecision", "latestReviews"} fragments := fmt.Sprintf(` fragment pr on PullRequest {%s} diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index e3fc92f5e..ba070908d 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -30,6 +30,7 @@ type StatusOptions struct { HasRepoOverride bool Exporter cmdutil.Exporter + ConflictStatus bool } func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { @@ -57,6 +58,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co }, } + cmd.Flags().BoolVarP(&opts.ConflictStatus, "conflict-status", "c", false, "Display the merge conflict status of each pull request") cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.PullRequestFields) return cmd @@ -91,13 +93,15 @@ func statusRun(opts *StatusOptions) error { } options := requestOptions{ - Username: "@me", - CurrentPR: currentPRNumber, - HeadRef: currentPRHeadRef, + Username: "@me", + CurrentPR: currentPRNumber, + HeadRef: currentPRHeadRef, + ConflictStatus: opts.ConflictStatus, } if opts.Exporter != nil { options.Fields = opts.Exporter.Fields() } + prPayload, err := pullRequestStatus(httpClient, baseRepo, options) if err != nil { return err @@ -264,6 +268,17 @@ func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { fmt.Fprint(w, cs.Green(fmt.Sprintf("✓ %s Approved", s))) } + if pr.Mergeable == "MERGEABLE" { + // prefer "No merge conflicts" to "Mergeable" as there is more to mergeability + // than the git status. Missing or failing required checks prevent merging + // even though a PR is technically mergeable, which is often a source of confusion. + fmt.Fprintf(w, " %s", cs.Green("✓ No merge conflicts")) + } else if pr.Mergeable == "CONFLICTING" { + fmt.Fprintf(w, " %s", cs.Red("× Merge conflicts")) + } else if pr.Mergeable == "UNKNOWN" { + fmt.Fprintf(w, " %s", cs.Yellow("! Merge conflict status unknown")) + } + if pr.BaseRef.BranchProtectionRule.RequiresStrictStatusChecks { switch pr.MergeStateStatus { case "BEHIND": diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go index 15bb17ab5..31f28396e 100644 --- a/pkg/cmd/pr/status/status_test.go +++ b/pkg/cmd/pr/status/status_test.go @@ -113,9 +113,10 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) { } expected := []string{ - "✓ Checks passing + Changes requested", + "✓ Checks passing + Changes requested ! Merge conflict status unknown", "- Checks pending ✓ 2 Approved", - "× 1/3 checks failing - Review required", + "× 1/3 checks failing - Review required ✓ No merge conflicts", + "✓ Checks passing × Merge conflicts", } for _, line := range expected {