From e8767b9706d523091da4d0f22713b893d68e202f Mon Sep 17 00:00:00 2001 From: lpessoa Date: Fri, 16 Sep 2022 13:47:50 -0300 Subject: [PATCH] feat: adding checks at GH PR view Fixes #6117 Adding checks to PR view as a new line along with changes information. Isolated 'status' display logic into a shared method in order to reuse it in 'view'. Updated existing 'view' tests. TODO: add new tests for PRs with checks. --- api/queries_pr.go | 1 + pkg/cmd/pr/shared/display.go | 16 ++++++++++++++++ pkg/cmd/pr/status/status.go | 13 +------------ pkg/cmd/pr/view/view.go | 17 +++++++++++++++-- pkg/cmd/pr/view/view_test.go | 15 ++++++++++----- 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 1774b579c..efcbd13d2 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -269,6 +269,7 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { } summary.Total++ } + return } diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index e31c86d2a..16194e039 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -73,3 +73,19 @@ func ListHeader(repoName string, itemName string, matchCount int, totalMatchCoun return fmt.Sprintf("Showing %d of %s in %s", matchCount, text.Pluralize(totalMatchCount, fmt.Sprintf("open %s", itemName)), repoName) } + +func PrCheckStatusSummaryWithColor(cs *iostreams.ColorScheme, checks api.PullRequestChecksStatus) string { + var summary string + if checks.Failing > 0 { + if checks.Failing == checks.Total { + summary = cs.Red("× All checks failing") + } else { + summary = cs.Redf("× %d/%d checks failing", checks.Failing, checks.Total) + } + } else if checks.Pending > 0 { + summary = cs.Yellow("- Checks pending") + } else if checks.Passing == checks.Total { + summary = cs.Green("✓ Checks passing") + } + return summary +} diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index e3fc92f5e..63a19d35f 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -230,18 +230,7 @@ func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { } if checks.Total > 0 { - var summary string - if checks.Failing > 0 { - if checks.Failing == checks.Total { - summary = cs.Red("× All checks failing") - } else { - summary = cs.Redf("× %d/%d checks failing", checks.Failing, checks.Total) - } - } else if checks.Pending > 0 { - summary = cs.Yellow("- Checks pending") - } else if checks.Passing == checks.Total { - summary = cs.Green("✓ Checks passing") - } + summary := shared.PrCheckStatusSummaryWithColor(cs, checks) fmt.Fprint(w, summary) } diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index a055922a8..10300a23c 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -81,7 +81,7 @@ var defaultFields = []string{ "isDraft", "maintainerCanModify", "mergeable", "additions", "deletions", "commitsCount", "baseRefName", "headRefName", "headRepositoryOwner", "headRepository", "isCrossRepository", "reviewRequests", "reviews", "assignees", "labels", "projectCards", "milestone", - "comments", "reactionGroups", "createdAt", + "comments", "reactionGroups", "createdAt", "statusCheckRollup", } func viewRun(opts *ViewOptions) error { @@ -171,17 +171,30 @@ func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error { // Header (Title and State) fmt.Fprintf(out, "%s #%d\n", cs.Bold(pr.Title), pr.Number) fmt.Fprintf(out, - "%s • %s wants to merge %s into %s from %s • %s • %s %s \n", + "%s • %s wants to merge %s into %s from %s • %s\n", shared.StateTitleWithColor(cs, *pr), pr.Author.Login, text.Pluralize(pr.Commits.TotalCount, "commit"), pr.BaseRefName, pr.HeadRefName, text.FuzzyAgo(opts.Now(), pr.CreatedAt), + ) + + // added/removed + fmt.Fprintf(out, + "%s %s", cs.Green("+"+strconv.Itoa(pr.Additions)), cs.Red("-"+strconv.Itoa(pr.Deletions)), ) + // checks + checks := pr.ChecksStatus() + if summary := shared.PrCheckStatusSummaryWithColor(cs, checks); summary != "" { + fmt.Fprintf(out, " • %s\n", summary) + } else { + fmt.Fprintln(out) + } + // Reactions if reactions := shared.ReactionGroupList(pr.ReactionGroups); reactions != "" { fmt.Fprint(out, reactions) diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 755e07542..7652845cd 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -353,7 +353,8 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork #12`, - `Open.*nobody wants to merge 12 commits into master from blueberries . about X years ago.+100.-10`, + `Open.*nobody wants to merge 12 commits into master from blueberries . about X years ago`, + `.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, @@ -366,7 +367,8 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork #12`, - `Open.*nobody wants to merge 12 commits into master from blueberries . about X years ago.+100.-10`, + `Open.*nobody wants to merge 12 commits into master from blueberries . about X years ago`, + `.+100.-10`, `Reviewers:.*1 \(.*Requested.*\)\n`, `Assignees:.*marseilles, monaco\n`, `Labels:.*one, two, three, four, five\n`, @@ -398,7 +400,8 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork #12`, - `Closed.*nobody wants to merge 12 commits into master from blueberries . about X years ago.+100.-10`, + `Closed.*nobody wants to merge 12 commits into master from blueberries . about X years ago`, + `.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, @@ -411,7 +414,8 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork #12`, - `Merged.*nobody wants to merge 12 commits into master from blueberries . about X years ago.+100.-10`, + `Merged.*nobody wants to merge 12 commits into master from blueberries . about X years ago`, + `.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, @@ -424,7 +428,8 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork #12`, - `Draft.*nobody wants to merge 12 commits into master from blueberries . about X years ago.+100.-10`, + `Draft.*nobody wants to merge 12 commits into master from blueberries . about X years ago`, + `.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, },