fix: address review findings
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 39s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 39s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m7s
- Gate inline comment resolution on successful supersede - Add pagination to ListReviewComments - Add tests for ListReviewComments and ResolveComment
This commit is contained in:
+25
-14
@@ -463,22 +463,33 @@ func (c *Client) EditComment(ctx context.Context, owner, repo string, commentID
|
||||
}
|
||||
|
||||
// ListReviewComments returns the inline comments attached to a specific review.
|
||||
// Paginates through all pages.
|
||||
func (c *Client) ListReviewComments(ctx context.Context, owner, repo string, prNumber int, reviewID int64) ([]ReviewComment, error) {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
prNumber,
|
||||
reviewID)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list review comments: %w", err)
|
||||
const pageSize = 50
|
||||
var all []ReviewComment
|
||||
for page := 1; ; page++ {
|
||||
reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d/comments?limit=%d&page=%d",
|
||||
c.baseURL,
|
||||
url.PathEscape(owner),
|
||||
url.PathEscape(repo),
|
||||
prNumber,
|
||||
reviewID,
|
||||
pageSize,
|
||||
page)
|
||||
body, err := c.doGet(ctx, reqURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list review comments (page %d): %w", page, err)
|
||||
}
|
||||
var batch []ReviewComment
|
||||
if err := json.Unmarshal(body, &batch); err != nil {
|
||||
return nil, fmt.Errorf("parse review comments (page %d): %w", page, err)
|
||||
}
|
||||
all = append(all, batch...)
|
||||
if len(batch) < pageSize {
|
||||
break
|
||||
}
|
||||
}
|
||||
var comments []ReviewComment
|
||||
if err := json.Unmarshal(body, &comments); err != nil {
|
||||
return nil, fmt.Errorf("parse review comments: %w", err)
|
||||
}
|
||||
return comments, nil
|
||||
return all, nil
|
||||
}
|
||||
|
||||
// ResolveComment marks an inline review comment as resolved.
|
||||
|
||||
@@ -602,3 +602,60 @@ func TestIsNotFound(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestListReviewComments(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprint(w, `[{"id":100,"path":"main.go","new_position":5,"body":"finding"},{"id":101,"path":"lib.go","new_position":10,"body":"another"}]`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
|
||||
if err != nil {
|
||||
t.Fatalf("ListReviewComments() error = %v", err)
|
||||
}
|
||||
if len(comments) != 2 {
|
||||
t.Fatalf("got %d comments, want 2", len(comments))
|
||||
}
|
||||
if comments[0].ID != 100 {
|
||||
t.Errorf("comments[0].ID = %d, want 100", comments[0].ID)
|
||||
}
|
||||
if comments[1].Path != "lib.go" {
|
||||
t.Errorf("comments[1].Path = %q, want %q", comments[1].Path, "lib.go")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveComment(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodPost {
|
||||
t.Errorf("expected POST, got %s", r.Method)
|
||||
}
|
||||
expected := "/api/v1/repos/owner/repo/pulls/comments/99/resolve"
|
||||
if r.URL.Path != expected {
|
||||
t.Errorf("path = %q, want %q", r.URL.Path, expected)
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveComment() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveComment_Error(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
fmt.Fprint(w, "not found")
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
err := client.ResolveComment(context.Background(), "owner", "repo", 99)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for 404 response")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user