From ffa5ce456adbe075dc8fd3b950db086a30c0c69e Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 5 Nov 2019 14:29:10 -0700 Subject: [PATCH 1/5] do not use color when stdout is not a terminal --- utils/color.go | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/utils/color.go b/utils/color.go index 6fe6f2fc5..fb8479734 100644 --- a/utils/color.go +++ b/utils/color.go @@ -1,24 +1,39 @@ package utils -import "github.com/mgutz/ansi" +import ( + "github.com/mattn/go-isatty" + "github.com/mgutz/ansi" + "os" +) -var Black = ansi.ColorFunc("black") -var White = ansi.ColorFunc("white") +func makeColorFunc(color string) func(string) string { + return func(arg string) string { + output := arg + if isatty.IsTerminal(os.Stdout.Fd()) { + output = ansi.Color(color+arg+ansi.Reset, "") + } -func Gray(arg string) string { - return ansi.Color(ansi.LightBlack+arg, "") + return output + } } -var Red = ansi.ColorFunc("red") -var Green = ansi.ColorFunc("green") -var Yellow = ansi.ColorFunc("yellow") -var Blue = ansi.ColorFunc("blue") -var Magenta = ansi.ColorFunc("magenta") -var Cyan = ansi.ColorFunc("cyan") +var Black = makeColorFunc(ansi.Black) +var White = makeColorFunc(ansi.White) +var Magenta = makeColorFunc(ansi.Magenta) +var Cyan = makeColorFunc(ansi.Cyan) +var Red = makeColorFunc(ansi.Red) +var Yellow = makeColorFunc(ansi.Yellow) +var Blue = makeColorFunc(ansi.Blue) +var Green = makeColorFunc(ansi.Green) +var Gray = makeColorFunc(ansi.LightBlack) func Bold(arg string) string { - // This is really annoying. If you just define Bold as ColorFunc("+b") it will properly bold but - // will not use the default color, resulting in black and probably unreadable text. This forces - // the default color before bolding. - return ansi.Color(ansi.DefaultFG+arg, "+b") + output := arg + if isatty.IsTerminal(os.Stdout.Fd()) { + // This is really annoying. If you just define Bold as ColorFunc("+b") it will properly bold but + // will not use the default color, resulting in black and probably unreadable text. This forces + // the default color before bolding. + output = ansi.Color(ansi.DefaultFG+arg+ansi.Reset, "+b") + } + return output } From 7419f7ae646868c6c9173c22ad25b9ab4cde2366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 17:07:27 +0100 Subject: [PATCH 2/5] Add checks, reviews info to `pr status` --- api/queries.go | 140 +++++++++++++++++++++++++++++++++++++++++++++--- command/pr.go | 20 ++++++- command/root.go | 2 + 3 files changed, 153 insertions(+), 9 deletions(-) diff --git a/api/queries.go b/api/queries.go index 5daf10244..78b34b7e7 100644 --- a/api/queries.go +++ b/api/queries.go @@ -17,6 +17,99 @@ type PullRequest struct { State string URL string HeadRefName string + Reviews struct { + Nodes []struct { + State string + Author struct { + Login string + } + } + } + Commits struct { + Nodes []struct { + Commit struct { + Status struct { + Contexts []struct { + State string + } + } + CheckSuites struct { + Nodes []struct { + CheckRuns struct { + Nodes []struct { + Conclusion string + } + } + } + } + } + } + } +} + +type PullRequestReviewStatus struct { + ChangesRequested bool + Approved bool +} + +func (pr *PullRequest) ReviewStatus() PullRequestReviewStatus { + status := PullRequestReviewStatus{} + reviewMap := map[string]string{} + // Reviews will include every review on record, including consecutive ones + // from the same actor. Consolidate them into latest state per reviewer. + for _, review := range pr.Reviews.Nodes { + reviewMap[review.Author.Login] = review.State + } + for _, state := range reviewMap { + switch state { + case "CHANGES_REQUESTED": + status.ChangesRequested = true + case "APPROVED": + status.Approved = true + } + } + return status +} + +type PullRequestChecksStatus struct { + Pending int + Failing int + Passing int + Total int +} + +func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { + if len(pr.Commits.Nodes) == 0 { + return + } + commit := pr.Commits.Nodes[0].Commit + for _, status := range commit.Status.Contexts { + switch status.State { + case "SUCCESS": + summary.Passing++ + case "EXPECTED", "ERROR", "FAILURE": + summary.Failing++ + case "PENDING": + summary.Pending++ + default: + panic(fmt.Errorf("unsupported status: %q", status.State)) + } + summary.Total++ + } + for _, checkSuite := range commit.CheckSuites.Nodes { + for _, checkRun := range checkSuite.CheckRuns.Nodes { + switch checkRun.Conclusion { + case "SUCCESS", "NEUTRAL": + summary.Passing++ + case "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": + summary.Failing++ + default: + panic(fmt.Errorf("unsupported check conclusion: %q", checkRun.Conclusion)) + } + summary.Total++ + } + } + return } type Repo interface { @@ -147,19 +240,50 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st } query := ` - fragment pr on PullRequest { - number - title - url - headRefName - } + fragment pr on PullRequest { + number + title + url + headRefName + commits(last: 1) { + nodes { + commit { + status { + contexts { + state + } + } + checkSuites(first: 50) { + nodes { + checkRuns(first: 50) { + nodes { + conclusion + } + } + } + } + } + } + } + } + fragment prWithReviews on PullRequest { + ...pr + reviews(last: 20) { + nodes { + state + author { + login + } + } + } + } query($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { pullRequests(headRefName: $headRefName, states: OPEN, first: 1) { edges { node { - ...pr + ...prWithReviews } } } @@ -167,7 +291,7 @@ func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername st viewerCreated: search(query: $viewerQuery, type: ISSUE, first: $per_page) { edges { node { - ...pr + ...prWithReviews } } pageInfo { diff --git a/command/pr.go b/command/pr.go index efbf8d995..0b9675db3 100644 --- a/command/pr.go +++ b/command/pr.go @@ -248,7 +248,25 @@ func prView(cmd *cobra.Command, args []string) error { func printPrs(prs ...api.PullRequest) { for _, pr := range prs { - fmt.Printf(" #%d %s %s\n", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + fmt.Printf(" #%d %s %s", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + if checks := pr.ChecksStatus(); checks.Total > 0 { + ratio := fmt.Sprintf("%d/%d", checks.Passing, checks.Total) + if checks.Failing > 0 { + ratio = utils.Red(ratio) + } else if checks.Pending > 0 { + ratio = utils.Yellow(ratio) + } else if checks.Passing == checks.Total { + ratio = utils.Green(ratio) + } + fmt.Printf(" - checks: %s", ratio) + } + reviews := pr.ReviewStatus() + if reviews.ChangesRequested { + fmt.Printf(" - %s", utils.Red("changes requested")) + } else if reviews.Approved { + fmt.Printf(" - %s", utils.Green("approved")) + } + fmt.Printf("\n") } } diff --git a/command/root.go b/command/root.go index 1e9c449b6..b3a95d878 100644 --- a/command/root.go +++ b/command/root.go @@ -72,6 +72,8 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { opts := []api.ClientOption{ api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), + // antiope-preview: Checks + api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), } if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, api.VerboseLog(os.Stderr)) From e1f52fed014f815a6103653359b9ea3829bb911c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 17:19:42 +0100 Subject: [PATCH 3/5] Display checks, reviews on a new line to prevent terminal wrap --- command/pr.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/command/pr.go b/command/pr.go index 0b9675db3..35b157df3 100644 --- a/command/pr.go +++ b/command/pr.go @@ -249,7 +249,14 @@ func prView(cmd *cobra.Command, args []string) error { func printPrs(prs ...api.PullRequest) { for _, pr := range prs { fmt.Printf(" #%d %s %s", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) - if checks := pr.ChecksStatus(); checks.Total > 0 { + + checks := pr.ChecksStatus() + reviews := pr.ReviewStatus() + if checks.Total > 0 || reviews.ChangesRequested || reviews.Approved { + fmt.Printf("\n ") + } + + if checks.Total > 0 { ratio := fmt.Sprintf("%d/%d", checks.Passing, checks.Total) if checks.Failing > 0 { ratio = utils.Red(ratio) @@ -260,12 +267,13 @@ func printPrs(prs ...api.PullRequest) { } fmt.Printf(" - checks: %s", ratio) } - reviews := pr.ReviewStatus() + if reviews.ChangesRequested { fmt.Printf(" - %s", utils.Red("changes requested")) } else if reviews.Approved { fmt.Printf(" - %s", utils.Green("approved")) } + fmt.Printf("\n") } } From f70accd3e0af0b5728421cf25ffeaf4c12b24059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 8 Nov 2019 22:03:47 +0100 Subject: [PATCH 4/5] `pr status` prints PR number in yellow This matches `pr list` --- command/pr.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 35b157df3..1103690d1 100644 --- a/command/pr.go +++ b/command/pr.go @@ -248,7 +248,8 @@ func prView(cmd *cobra.Command, args []string) error { func printPrs(prs ...api.PullRequest) { for _, pr := range prs { - fmt.Printf(" #%d %s %s", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + prNumber := fmt.Sprintf("#%d", pr.Number) + fmt.Printf(" %s %s %s", utils.Yellow(prNumber), truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) checks := pr.ChecksStatus() reviews := pr.ReviewStatus() From 8fea48146dcd6845c0f4b8b390e5c38cc464c0aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 8 Nov 2019 22:04:18 +0100 Subject: [PATCH 5/5] Shorter representation of successful checks in `pr status` --- command/pr.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/command/pr.go b/command/pr.go index 1103690d1..037e2892b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -258,12 +258,15 @@ func printPrs(prs ...api.PullRequest) { } if checks.Total > 0 { - ratio := fmt.Sprintf("%d/%d", checks.Passing, checks.Total) + var ratio string if checks.Failing > 0 { + ratio = fmt.Sprintf("%d/%d", checks.Passing, checks.Total) ratio = utils.Red(ratio) } else if checks.Pending > 0 { + ratio = fmt.Sprintf("%d/%d", checks.Passing, checks.Total) ratio = utils.Yellow(ratio) } else if checks.Passing == checks.Total { + ratio = fmt.Sprintf("%d", checks.Total) ratio = utils.Green(ratio) } fmt.Printf(" - checks: %s", ratio)