X Tutup
Skip to content

Commit 5ba3baa

Browse files
author
Nate Smith
authored
Merge pull request cli#762 from doi-t/reviewers-in-pr-view
Add Reviewers to pr view in CLI
2 parents 07dc617 + d2d0b47 commit 5ba3baa

File tree

5 files changed

+279
-10
lines changed

5 files changed

+279
-10
lines changed

‎api/queries_pr.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"fmt"
66
"strings"
7-
"time"
87

98
"github.com/shurcooL/githubv4"
109

@@ -86,6 +85,7 @@ type PullRequest struct {
8685
RequestedReviewer struct {
8786
TypeName string `json:"__typename"`
8887
Login string
88+
Name string
8989
}
9090
}
9191
TotalCount int
@@ -95,9 +95,7 @@ type PullRequest struct {
9595
Author struct {
9696
Login string
9797
}
98-
State string
99-
CreatedAt time.Time
100-
PublishedAt time.Time
98+
State string
10199
}
102100
}
103101
Assignees struct {
@@ -403,6 +401,9 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
403401
...on User {
404402
login
405403
}
404+
...on Team {
405+
name
406+
}
406407
}
407408
}
408409
totalCount
@@ -413,8 +414,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu
413414
login
414415
}
415416
state
416-
createdAt
417-
publishedAt
418417
}
419418
totalCount
420419
}
@@ -503,6 +502,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
503502
...on User {
504503
login
505504
}
505+
...on Team {
506+
name
507+
}
506508
}
507509
}
508510
totalCount
@@ -513,8 +515,6 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea
513515
login
514516
}
515517
state
516-
createdAt
517-
publishedAt
518518
}
519519
totalCount
520520
}

‎command/pr.go‎

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"io"
66
"regexp"
7+
"sort"
78
"strconv"
89
"strings"
910

@@ -429,10 +430,13 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error {
429430
pr.BaseRefName,
430431
pr.HeadRefName,
431432
)))
433+
fmt.Fprintln(out)
432434

433435
// Metadata
434-
// TODO: Reviewers
435-
fmt.Fprintln(out)
436+
if reviewers := prReviewerList(*pr); reviewers != "" {
437+
fmt.Fprint(out, utils.Bold("Reviewers: "))
438+
fmt.Fprintln(out, reviewers)
439+
}
436440
if assignees := prAssigneeList(*pr); assignees != "" {
437441
fmt.Fprint(out, utils.Bold("Assignees: "))
438442
fmt.Fprintln(out, assignees)
@@ -466,6 +470,116 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error {
466470
return nil
467471
}
468472

473+
// Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/
474+
const (
475+
requestedReviewState = "REQUESTED" // This is our own state for review request
476+
approvedReviewState = "APPROVED"
477+
changesRequestedReviewState = "CHANGES_REQUESTED"
478+
commentedReviewState = "COMMENTED"
479+
)
480+
481+
type reviewerState struct {
482+
Name string
483+
State string
484+
}
485+
486+
// colorFuncForReviewerState returns a color function for a reviewer state
487+
func colorFuncForReviewerState(state string) func(string) string {
488+
switch state {
489+
case requestedReviewState:
490+
return utils.Yellow
491+
case approvedReviewState:
492+
return utils.Green
493+
case changesRequestedReviewState:
494+
return utils.Red
495+
case commentedReviewState:
496+
return func(str string) string { return str } // Do nothing
497+
default:
498+
return nil
499+
}
500+
}
501+
502+
// formattedReviewerState formats a reviewerState with state color
503+
func formattedReviewerState(reviewer *reviewerState) string {
504+
stateColorFunc := colorFuncForReviewerState(reviewer.State)
505+
return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " ")))
506+
}
507+
508+
// prReviewerList generates a reviewer list with their last state
509+
func prReviewerList(pr api.PullRequest) string {
510+
reviewerStates := parseReviewers(pr)
511+
reviewers := make([]string, 0, len(reviewerStates))
512+
513+
sortReviewerStates(reviewerStates)
514+
515+
for _, reviewer := range reviewerStates {
516+
reviewers = append(reviewers, formattedReviewerState(reviewer))
517+
}
518+
519+
reviewerList := strings.Join(reviewers, ", ")
520+
521+
return reviewerList
522+
}
523+
524+
// Ref. https://developer.github.com/v4/union/requestedreviewer/
525+
const teamTypeName = "Team"
526+
527+
const ghostName = "ghost"
528+
529+
// parseReviewers parses given Reviews and ReviewRequests
530+
func parseReviewers(pr api.PullRequest) []*reviewerState {
531+
reviewerStates := make(map[string]*reviewerState)
532+
533+
for _, review := range pr.Reviews.Nodes {
534+
if review.Author.Login != pr.Author.Login {
535+
name := review.Author.Login
536+
if name == "" {
537+
name = ghostName
538+
}
539+
reviewerStates[name] = &reviewerState{
540+
Name: name,
541+
State: review.State,
542+
}
543+
}
544+
}
545+
546+
// Overwrite reviewer's state if a review request for the same reviewer exists.
547+
for _, reviewRequest := range pr.ReviewRequests.Nodes {
548+
name := reviewRequest.RequestedReviewer.Login
549+
if reviewRequest.RequestedReviewer.TypeName == teamTypeName {
550+
name = reviewRequest.RequestedReviewer.Name
551+
}
552+
reviewerStates[name] = &reviewerState{
553+
Name: name,
554+
State: requestedReviewState,
555+
}
556+
}
557+
558+
// Convert map to slice for ease of sort
559+
result := make([]*reviewerState, 0, len(reviewerStates))
560+
for _, reviewer := range reviewerStates {
561+
result = append(result, reviewer)
562+
}
563+
564+
return result
565+
}
566+
567+
// sortReviewerStates puts completed reviews before review requests and sorts names alphabetically
568+
func sortReviewerStates(reviewerStates []*reviewerState) {
569+
sort.Slice(reviewerStates, func(i, j int) bool {
570+
if reviewerStates[i].State == requestedReviewState &&
571+
reviewerStates[j].State != requestedReviewState {
572+
return false
573+
}
574+
if reviewerStates[j].State == requestedReviewState &&
575+
reviewerStates[i].State != requestedReviewState {
576+
return true
577+
}
578+
579+
return reviewerStates[i].Name < reviewerStates[j].Name
580+
})
581+
}
582+
469583
func prAssigneeList(pr api.PullRequest) string {
470584
if len(pr.Assignees.Nodes) == 0 {
471585
return ""

‎command/pr_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ func TestPRView_Preview(t *testing.T) {
461461
expectedOutputs: []string{
462462
`Blueberries are from a fork`,
463463
`Open • nobody wants to merge 12 commits into master from blueberries`,
464+
`Reviewers: 2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`,
464465
`Assignees: marseilles, monaco\n`,
465466
`Labels: one, two, three, four, five\n`,
466467
`Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
@@ -469,6 +470,17 @@ func TestPRView_Preview(t *testing.T) {
469470
`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`,
470471
},
471472
},
473+
"Open PR with reviewers by number": {
474+
ownerRepo: "master",
475+
args: "pr view 12",
476+
fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json",
477+
expectedOutputs: []string{
478+
`Blueberries are from a fork`,
479+
`Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`,
480+
`blueberries taste good`,
481+
`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`,
482+
},
483+
},
472484
"Open PR with metadata by branch": {
473485
ownerRepo: "master",
474486
args: "pr view blueberries",

‎test/fixtures/prViewPreviewWithMetadataByNumber.json‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,39 @@
1010
"author": {
1111
"login": "nobody"
1212
},
13+
"reviewRequests": {
14+
"nodes": [
15+
{
16+
"requestedReviewer": {
17+
"__typename": "User",
18+
"login": "1"
19+
}
20+
}
21+
],
22+
"totalcount": 1
23+
},
24+
"reviews": {
25+
"nodes": [
26+
{
27+
"author": {
28+
"login": "3"
29+
},
30+
"state": "COMMENTED"
31+
},
32+
{
33+
"author": {
34+
"login": "2"
35+
},
36+
"state": "APPROVED"
37+
},
38+
{
39+
"author": {
40+
"login": "1"
41+
},
42+
"state": "CHANGES_REQUESTED"
43+
}
44+
]
45+
},
1346
"assignees": {
1447
"nodes": [
1548
{

‎test/fixtures/prViewPreviewWithReviewersByNumber.json‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
{
2+
"data": {
3+
"repository": {
4+
"pullRequest": {
5+
"number": 12,
6+
"title": "Blueberries are from a fork",
7+
"state": "OPEN",
8+
"body": "**blueberries taste good**",
9+
"url": "https://github.com/OWNER/REPO/pull/12",
10+
"author": {
11+
"login": "nobody"
12+
},
13+
"reviewRequests": {
14+
"nodes": [
15+
{
16+
"requestedReviewer": {
17+
"__typename": "user",
18+
"login": "123"
19+
}
20+
},
21+
{
22+
"requestedReviewer": {
23+
"__typename": "Team",
24+
"name": "Team 1"
25+
}
26+
},
27+
{
28+
"requestedReviewer": {
29+
"__typename": "user",
30+
"login": "abc"
31+
}
32+
}
33+
],
34+
"totalcount": 1
35+
},
36+
"reviews": {
37+
"nodes": [
38+
{
39+
"author": {
40+
"login": "123"
41+
},
42+
"state": "COMMENTED"
43+
},
44+
{
45+
"author": {
46+
"login": "def"
47+
},
48+
"state": "CHANGES_REQUESTED"
49+
},
50+
{
51+
"author": {
52+
"login": "abc"
53+
},
54+
"state": "APPROVED"
55+
},
56+
{
57+
"author": {
58+
"login": "DEF"
59+
},
60+
"state": "COMMENTED"
61+
},
62+
{
63+
"author": {
64+
"login": "xyz"
65+
},
66+
"state": "APPROVED"
67+
},
68+
{
69+
"author": {
70+
"login": ""
71+
},
72+
"state": "APPROVED"
73+
}
74+
]
75+
},
76+
"assignees": {
77+
"nodes": [],
78+
"totalcount": 0
79+
},
80+
"labels": {
81+
"nodes": [],
82+
"totalcount": 0
83+
},
84+
"projectcards": {
85+
"nodes": [],
86+
"totalcount": 0
87+
},
88+
"milestone": {},
89+
"participants": {
90+
"nodes": [
91+
{
92+
"login": "marseilles"
93+
}
94+
],
95+
"totalcount": 1
96+
},
97+
"commits": {
98+
"totalCount": 12
99+
},
100+
"baseRefName": "master",
101+
"headRefName": "blueberries",
102+
"headRepositoryOwner": {
103+
"login": "hubot"
104+
},
105+
"isCrossRepository": true,
106+
"isDraft": false
107+
}
108+
}
109+
}
110+
}

0 commit comments

Comments
 (0)
X Tutup