diff --git a/command/pr_review.go b/command/pr_review.go index 1ecc7f8bc..bd0453784 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -13,22 +13,25 @@ import ( func init() { prCmd.AddCommand(prReviewCmd) - prReviewCmd.Flags().StringP("approve", "a", "", "Approve pull request") - prReviewCmd.Flags().StringP("request-changes", "r", "", "Request changes on a pull request") - prReviewCmd.Flags().StringP("comment", "c", "", "Comment on a pull request") - - // this is required; without it pflag complains that you must pass a string to string flags. - prReviewCmd.Flags().Lookup("approve").NoOptDefVal = " " - prReviewCmd.Flags().Lookup("request-changes").NoOptDefVal = " " - prReviewCmd.Flags().Lookup("comment").NoOptDefVal = " " + prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request") + prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request") + prReviewCmd.Flags().BoolP("comment", "c", false, "Comment on a pull request") + prReviewCmd.Flags().StringP("body", "b", "", "Specify the body of a review") } var prReviewCmd = &cobra.Command{ - Use: "review", - Short: "TODO", + Use: "review [{ | | ]", + Short: "Add a review to a pull request.", Args: cobra.MaximumNArgs(1), - Long: "TODO", - RunE: prReview, + Long: `Add a review to either a specified pull request or the pull request associated with the current branch. + +Examples: + + gh pr review -a # mark the current branch's pull request as approved + gh pr review -c -b "interesting" # comment on the current branch's pull request + gh pr review 123 -r -b "needs more ascii art" # request changes on pull request 123 + `, + RunE: prReview, } func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { @@ -56,18 +59,13 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { return nil, errors.New("need exactly one of --approve, --request-changes, or --comment") } - val, err := cmd.Flags().GetString(flag) + body, err := cmd.Flags().GetString("body") if err != nil { return nil, err } - body := "" - if val != " " { - body = val - } - - if flag == "comment" && (body == " " || len(body) == 0) { - return nil, errors.New("cannot leave blank comment") + if (flag == "request-changes" || flag == "comment") && body == "" { + return nil, fmt.Errorf("body cannot be blank for %s review", flag) } return &api.PullRequestReviewInput{ @@ -100,13 +98,12 @@ func prReview(cmd *cobra.Command, args []string) error { prArg, repo := prFromURL(args[0]) if repo != nil { baseRepo = repo - // TODO handle malformed URL; it falls through to Atoi } else { prArg = strings.TrimPrefix(args[0], "#") } prNum, err = strconv.Atoi(prArg) if err != nil { - return fmt.Errorf("could not parse pull request number: %w", err) + return errors.New("could not parse pull request argument") } } diff --git a/command/pr_review_test.go b/command/pr_review_test.go index 50580450d..96ceabd7e 100644 --- a/command/pr_review_test.go +++ b/command/pr_review_test.go @@ -13,7 +13,7 @@ func TestPRReview_validation(t *testing.T) { for _, cmd := range []string{ `pr review 123`, `pr review --approve --comment 123`, - `pr review --approve="cool" --comment="rad" 123`, + `pr review --approve --comment -b"hey" 123`, } { http.StubRepoResponse("OWNER", "REPO") _, err := RunCommand(cmd) @@ -89,7 +89,7 @@ func TestPRReview_number_arg(t *testing.T) { } } } } `)) http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) - _, err := RunCommand("pr review --request-changes 123") + _, err := RunCommand("pr review --approve 123") if err != nil { t.Fatalf("error running pr review: %s", err) } @@ -107,7 +107,7 @@ func TestPRReview_number_arg(t *testing.T) { _ = json.Unmarshal(bodyBytes, &reqBody) eq(t, reqBody.Variables.Input.PullRequestID, "foobar123") - eq(t, reqBody.Variables.Input.Event, "REQUEST_CHANGES") + eq(t, reqBody.Variables.Input.Event, "APPROVE") eq(t, reqBody.Variables.Input.Body, "") } @@ -121,11 +121,10 @@ func TestPRReview_no_arg(t *testing.T) { "id": "foobar123", "headRefName": "feature", "baseRefName": "master" } - ] } } } } - `)) + ] } } } }`)) http.StubResponse(200, bytes.NewBufferString(`{"data": {} }`)) - _, err := RunCommand(`pr review --comment="cool story"`) + _, err := RunCommand(`pr review --comment -b "cool story"`) if err != nil { t.Fatalf("error running pr review: %s", err) } @@ -153,7 +152,16 @@ func TestPRReview_blank_comment(t *testing.T) { http.StubRepoResponse("OWNER", "REPO") _, err := RunCommand(`pr review --comment 123`) - eq(t, err.Error(), "did not understand desired review action: cannot leave blank comment") + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for comment review") +} + +func TestPRReview_blank_request_changes(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr review -r 123`) + eq(t, err.Error(), "did not understand desired review action: body cannot be blank for request-changes review") } func TestPRReview(t *testing.T) { @@ -163,11 +171,10 @@ func TestPRReview(t *testing.T) { ExpectedBody string } cases := []c{ - c{`pr review --request-changes="bad"`, "REQUEST_CHANGES", "bad"}, - c{`pr review --request-changes`, "REQUEST_CHANGES", ""}, + c{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, c{`pr review --approve`, "APPROVE", ""}, - c{`pr review --approve="hot damn"`, "APPROVE", "hot damn"}, - c{`pr review --comment="i donno"`, "COMMENT", "i donno"}, + c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"}, + c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"}, } for _, kase := range cases {