switch to using body flag. small TODOs

command/pr_review.go

@@ -13,22 +13,25 @@ import (
 func init() {
 	prCmd.AddCommand(prReviewCmd)
 
-	prReviewCmd.Flags().StringP("approve", "a", "", "Approve pull request")
+	prReviewCmd.Flags().BoolP("approve", "a", false, "Approve pull request")
-	prReviewCmd.Flags().StringP("request-changes", "r", "", "Request changes on a pull request")
+	prReviewCmd.Flags().BoolP("request-changes", "r", false, "Request changes on a pull request")
-	prReviewCmd.Flags().StringP("comment", "c", "", "Comment 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")
-	// 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 = " "
 }
 
 var prReviewCmd = &cobra.Command{
-	Use:   "review",
+	Use:   "review [{<number> | <url> | <branch>]",
-	Short: "TODO",
+	Short: "Add a review to a pull request.",
 	Args:  cobra.MaximumNArgs(1),
-	Long:  "TODO",
+	Long: `Add a review to either a specified pull request or the pull request associated with the current branch.
-	RunE:  prReview,
+
+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 (flag == "request-changes" || flag == "comment") && body == "" {
-	if val != " " {
+		return nil, fmt.Errorf("body cannot be blank for %s review", flag)
-		body = val
-	}
-
-	if flag == "comment" && (body == " " || len(body) == 0) {
-		return nil, errors.New("cannot leave blank comment")
 	}
 
 	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")
 		}
 	}
 

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 -b"bad"`, "REQUEST_CHANGES", "bad"},
-		c{`pr review --request-changes`, "REQUEST_CHANGES", ""},
 		c{`pr review --approve`, "APPROVE", ""},
-		c{`pr review --approve="hot damn"`, "APPROVE", "hot damn"},
+		c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"},
-		c{`pr review --comment="i donno"`, "COMMENT", "i donno"},
+		c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"},
 	}
 
 	for _, kase := range cases {