Skip to content

Commit 6538cca

Browse files
vilmibmvilmibm
committed
switch to using body flag. small TODOs
1 parent c40f064 commit 6538cca

File tree

2 files changed

+37
-33
lines changed

2 files changed

+37
-33
lines changed

‎command/pr_review.go‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,25 @@ import (
1313
func init() {
1414
prCmd.AddCommand(prReviewCmd)
1515

16-
prReviewCmd.Flags().StringP("approve", "a", "", "Approve pull request")
17-
prReviewCmd.Flags().StringP("request-changes", "r", "", "Request changes on a pull request")
18-
prReviewCmd.Flags().StringP("comment", "c", "", "Comment on a pull request")
19-
20-
// this is required; without it pflag complains that you must pass a string to string flags.
21-
prReviewCmd.Flags().Lookup("approve").NoOptDefVal = " "
22-
prReviewCmd.Flags().Lookup("request-changes").NoOptDefVal = " "
23-
prReviewCmd.Flags().Lookup("comment").NoOptDefVal = " "
16+
prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request")
17+
prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request")
18+
prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request")
19+
prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review")
2420
}
2521

2622
var prReviewCmd = &cobra.Command{
27-
Use: "review",
28-
Short: "TODO",
23+
Use: "review [{<number> | <url> | <branch>]",
24+
Short: "Add a review to a pull request.",
2925
Args: cobra.MaximumNArgs(1),
30-
Long: "TODO",
31-
RunE: prReview,
26+
Long: `Add a review to either a specified pull request or the pull request associated with the current branch.
27+
28+
Examples:
29+
30+
gh pr review -a # mark the current branch's pull request as approved
31+
gh pr review -c -b "interesting" # comment on the current branch's pull request
32+
gh pr review 123 -r -b "needs more ascii art" # request changes on pull request 123
33+
`,
34+
RunE: prReview,
3235
}
3336

3437
func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
@@ -56,18 +59,13 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) {
5659
return nil, errors.New("need exactly one of --approve, --request-changes, or --comment")
5760
}
5861

59-
val, err := cmd.Flags().GetString(flag)
62+
body, err := cmd.Flags().GetString("body")
6063
if err != nil {
6164
return nil, err
6265
}
6366

64-
body := ""
65-
if val != " " {
66-
body = val
67-
}
68-
69-
if flag == "comment" && (body == " " || len(body) == 0) {
70-
return nil, errors.New("cannot leave blank comment")
67+
if (flag == "request-changes" || flag == "comment") && body == "" {
68+
return nil, fmt.Errorf("body cannot be blank for %s review", flag)
7169
}
7270

7371
return &api.PullRequestReviewInput{
@@ -100,13 +98,12 @@ func prReview(cmd *cobra.Command, args []string) error {
10098
prArg, repo := prFromURL(args[0])
10199
if repo != nil {
102100
baseRepo = repo
103-
// TODO handle malformed URL; it falls through to Atoi
104101
} else {
105102
prArg = strings.TrimPrefix(args[0], "#")
106103
}
107104
prNum, err = strconv.Atoi(prArg)
108105
if err != nil {
109-
return fmt.Errorf("could not parse pull request number: %w", err)
106+
return errors.New("could not parse pull request argument")
110107
}
111108
}
112109

‎command/pr_review_test.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func TestPRReview_validation(t *testing.T) {
1313
for _, cmd := range []string{
1414
`pr review 123`,
1515
`pr review --approve --comment 123`,
16-
`pr review --approve="cool" --comment="rad" 123`,
16+
`pr review --approve --comment -b"hey" 123`,
1717
} {
1818
http.StubRepoResponse("OWNER", "REPO")
1919
_, err := RunCommand(cmd)
@@ -89,7 +89,7 @@ func TestPRReview_number_arg(t *testing.T) {
8989
} } } } `))
9090
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
9191

92-
_, err := RunCommand("pr review --request-changes 123")
92+
_, err := RunCommand("pr review --approve 123")
9393
if err != nil {
9494
t.Fatalf("error running pr review: %s", err)
9595
}
@@ -107,7 +107,7 @@ func TestPRReview_number_arg(t *testing.T) {
107107
_ = json.Unmarshal(bodyBytes, &reqBody)
108108

109109
eq(t, reqBody.Variables.Input.PullRequestID, "foobar123")
110-
eq(t, reqBody.Variables.Input.Event, "REQUEST_CHANGES")
110+
eq(t, reqBody.Variables.Input.Event, "APPROVE")
111111
eq(t, reqBody.Variables.Input.Body, "")
112112
}
113113

@@ -121,11 +121,10 @@ func TestPRReview_no_arg(t *testing.T) {
121121
"id": "foobar123",
122122
"headRefName": "feature",
123123
"baseRefName": "master" }
124-
] } } } }
125-
`))
124+
] } } } }`))
126125
http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`))
127126

128-
_, err := RunCommand(`pr review --comment="cool story"`)
127+
_, err := RunCommand(`pr review --comment -b "cool story"`)
129128
if err != nil {
130129
t.Fatalf("error running pr review: %s", err)
131130
}
@@ -153,7 +152,16 @@ func TestPRReview_blank_comment(t *testing.T) {
153152
http.StubRepoResponse("OWNER", "REPO")
154153

155154
_, err := RunCommand(`pr review --comment 123`)
156-
eq(t, err.Error(), "did not understand desired review action: cannot leave blank comment")
155+
eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review")
156+
}
157+
158+
func TestPRReview_blank_request_changes(t *testing.T) {
159+
initBlankContext("", "OWNER/REPO", "master")
160+
http := initFakeHTTP()
161+
http.StubRepoResponse("OWNER", "REPO")
162+
163+
_, err := RunCommand(`pr review -r 123`)
164+
eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review")
157165
}
158166

159167
func TestPRReview(t *testing.T) {
@@ -163,11 +171,10 @@ func TestPRReview(t *testing.T) {
163171
ExpectedBody string
164172
}
165173
cases := []c{
166-
c{`pr review --request-changes="bad"`, "REQUEST_CHANGES", "bad"},
167-
c{`pr review --request-changes`, "REQUEST_CHANGES", ""},
174+
c{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"},
168175
c{`pr review --approve`, "APPROVE", ""},
169-
c{`pr review --approve="hot damn"`, "APPROVE", "hot damn"},
170-
c{`pr review --comment="i donno"`, "COMMENT", "i donno"},
176+
c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"},
177+
c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"},
171178
}
172179

173180
for _, kase := range cases {

0 commit comments

Comments
 (0)