From 8413665638345fe950a609dfccba3341282cb816 Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Wed, 13 May 2026 01:29:14 -0700
Subject: [PATCH] fix(github): clarify PostReview doc comment, rename test
 field to 'want'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address review feedback from round-3 sonnet review:
- PostReview doc comment now accurately describes vcs.ReviewEvent → GitHub
  wire-format string cast and notes nil-Comments omitempty behavior.
- Rename 'expected' field to 'want' in TestTranslateGitHubReviewState to
  match the project's established naming convention.
---
 github/review.go      | 10 ++++++++--
 github/review_test.go |  8 ++++----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/github/review.go b/github/review.go
index 7502fc7..ead197f 100644
--- a/github/review.go
+++ b/github/review.go
@@ -66,9 +66,15 @@ func translateGitHubReviewState(state string) string {
 }
 
 // PostReview submits a review on a pull request.
-// The ReviewRequest.Event values (APPROVE, REQUEST_CHANGES, COMMENT) are sent
-// directly — they match GitHub's canonical event strings.
+//
+// The vcs.ReviewEvent constants (ReviewEventApprove, ReviewEventRequestChanges,
+// ReviewEventComment) have string values that match GitHub's wire-format event
+// strings (APPROVE, REQUEST_CHANGES, COMMENT), so Event is cast directly to
+// string without translation.
+//
 // ReviewComment.Position maps directly to the GitHub API position field.
+// When req.Comments is empty, the payload omits the comments field entirely
+// (via the omitempty tag on postReviewRequest.Comments).
 func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
 	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
 		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
diff --git a/github/review_test.go b/github/review_test.go
index da6535d..fa1200c 100644
--- a/github/review_test.go
+++ b/github/review_test.go
@@ -362,8 +362,8 @@ func TestDismissReview_401(t *testing.T) {
 
 func TestTranslateGitHubReviewState(t *testing.T) {
 	tests := []struct {
-		input    string
-		expected string
+		input string
+		want  string
 	}{
 		{"APPROVED", "APPROVED"},
 		{"CHANGES_REQUESTED", "REQUEST_CHANGES"},
@@ -374,8 +374,8 @@ func TestTranslateGitHubReviewState(t *testing.T) {
 	}
 	for _, tt := range tests {
 		got := translateGitHubReviewState(tt.input)
-		if got != tt.expected {
-			t.Errorf("translateGitHubReviewState(%q) = %q, want %q", tt.input, got, tt.expected)
+		if got != tt.want {
+			t.Errorf("translateGitHubReviewState(%q) = %q, want %q", tt.input, got, tt.want)
 		}
 	}
 }