rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

feat(github): implement Reviewer and Identity interfaces (#81) #105

Merged
aweiker merged 7 commits from review-bot-issue-81 into feature/github-support 2026-05-13 13:39:14 +00:00
Conversation 40 Commits 7 Files Changed 7
+11 -9
2 changed files with 11 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 027bad2f7c - Show all commits
github/review.go
+3 -1
View File
15
 @@ -194,7 +194,9 @@ func (c *Client) DismissReview(ctx context.Context, owner, repo string, number i
payload := dismissReviewRequest{ payload := dismissReviewRequest{
sonnet-review-bot commented 2026-05-13 09:05:11 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] The DismissReview method hardcodes Event: "DISMISS" in the payload struct. Since this is the only valid value for a dismissal, it's not a bug, but a small comment noting why the field is included (GitHub API requires it) would improve readability for future maintainers.

**[NIT]** The `DismissReview` method hardcodes `Event: "DISMISS"` in the payload struct. Since this is the only valid value for a dismissal, it's not a bug, but a small comment noting why the field is included (GitHub API requires it) would improve readability for future maintainers.
rodin commented 2026-05-13 10:07:09 +00:00
Outdated
Review
Copy Link
Copy Source

Fixed in dbc25f4 — added a comment explaining that the GitHub API requires the Event field for dismissal requests even though DISMISS is the only valid value.

Fixed in dbc25f4 — added a comment explaining that the GitHub API requires the `Event` field for dismissal requests even though `DISMISS` is the only valid value.
Message: message, Message: message,
Event: "DISMISS", // Event is required by the GitHub API for dismissal requests, even though
// "DISMISS" is the only valid value for this endpoint.
Event: "DISMISS",
} }
data, err := json.Marshal(payload) data, err := json.Marshal(payload)
github/review_test.go
+8 -8
View File
6
 @@ -17,10 +17,10 @@ import (
func TestPostReview_HappyPath(t *testing.T) { func TestPostReview_HappyPath(t *testing.T) {
c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != "POST" { if r.Method != "POST" {
t.Errorf("expected POST, got %s", r.Method) t.Fatalf("expected POST, got %s", r.Method)
} }
if r.URL.Path != "/repos/owner/repo/pulls/5/reviews" { if r.URL.Path != "/repos/owner/repo/pulls/5/reviews" {
t.Errorf("unexpected path: %s", r.URL.Path) t.Fatalf("unexpected path: %s", r.URL.Path)
} }
if r.Header.Get("Content-Type") != "application/json" { if r.Header.Get("Content-Type") != "application/json" {
t.Errorf("expected Content-Type application/json, got %q", r.Header.Get("Content-Type")) t.Errorf("expected Content-Type application/json, got %q", r.Header.Get("Content-Type"))
1
 @@ -150,10 +150,10 @@ func TestPostReview_MalformedResponse(t *testing.T) {
func TestListReviews_HappyPath(t *testing.T) { func TestListReviews_HappyPath(t *testing.T) {
c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != "GET" { if r.Method != "GET" {
t.Errorf("expected GET, got %s", r.Method) t.Fatalf("expected GET, got %s", r.Method)
} }
if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" { if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" {
t.Errorf("unexpected path: %s", r.URL.Path) t.Fatalf("unexpected path: %s", r.URL.Path)
} }
json.NewEncoder(w).Encode([]map[string]interface{}{ json.NewEncoder(w).Encode([]map[string]interface{}{
{ {
@@ -250,10 +250,10 @@ func TestListReviews_401(t *testing.T) {
func TestDeleteReview_HappyPath(t *testing.T) { func TestDeleteReview_HappyPath(t *testing.T) {
c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != "DELETE" { if r.Method != "DELETE" {
t.Errorf("expected DELETE, got %s", r.Method) t.Fatalf("expected DELETE, got %s", r.Method)
} }
if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/42" { if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/42" {
t.Errorf("unexpected path: %s", r.URL.Path) t.Fatalf("unexpected path: %s", r.URL.Path)
} }
w.WriteHeader(204) w.WriteHeader(204)
}) })
@@ -284,10 +284,10 @@ func TestDeleteReview_422_SubmittedReview(t *testing.T) {
func TestDismissReview_HappyPath(t *testing.T) { func TestDismissReview_HappyPath(t *testing.T) {
c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if r.Method != "PUT" { if r.Method != "PUT" {
t.Errorf("expected PUT, got %s", r.Method) t.Fatalf("expected PUT, got %s", r.Method)
} }
if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/10/dismissals" { if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/10/dismissals" {
t.Errorf("unexpected path: %s", r.URL.Path) t.Fatalf("unexpected path: %s", r.URL.Path)
} }
body, _ := io.ReadAll(r.Body) body, _ := io.ReadAll(r.Body)
4