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

fix(github): consolidate review.go and identity.go into reviews.go (#116) #119

Merged
aweiker merged 4 commits from review-bot-issue-116 into feature/github-support 2026-05-14 01:49:57 +00:00
Conversation 27 Commits 4 Files Changed 4
+2 -1
2 changed files with 2 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 2e2fcbabfc - Show all commits
github/review_test.go
+1 -1
View File
@@ -2,9 +2,9 @@ package github
import (
"context"
sonnet-review-bot commented 2026-05-13 23:03:57 +00:00
Review
Copy Link
Copy Source

[NIT] Import ordering: fmt is listed before encoding/json, breaking the standard goimports ordering (stdlib imports should be alphabetical within their group). gofmt/goimports would reorder these. Minor since CI passes, but worth fixing for consistency with the style pattern.

**[NIT]** Import ordering: `fmt` is listed before `encoding/json`, breaking the standard goimports ordering (stdlib imports should be alphabetical within their group). gofmt/goimports would reorder these. Minor since CI passes, but worth fixing for consistency with the style pattern.
sonnet-review-bot commented 2026-05-14 00:23:41 +00:00
Review
Copy Link
Copy Source

[NIT] Import ordering: fmt is inserted before encoding/json but goimports/gofmt would order stdlib imports alphabetically (context, encoding/json, errors, fmt, io, net/http, strings, testing). This is a minor style deviation that goimports would fix automatically.

**[NIT]** Import ordering: `fmt` is inserted before `encoding/json` but goimports/gofmt would order stdlib imports alphabetically (`context`, `encoding/json`, `errors`, `fmt`, `io`, `net/http`, `strings`, `testing`). This is a minor style deviation that `goimports` would fix automatically.
"fmt"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"strings"
github/reviews.go
+1
View File
6
 @@ -221,6 +221,7 @@ func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number in
reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d",
sonnet-review-bot commented 2026-05-14 00:23:41 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] The comment // nil body: the GitHub DELETE endpoint for reviews requires no request body. was removed in the diff. It was a useful clarification for why nil is passed to doRequestWithBody. Consider retaining it or replacing with an equivalent inline comment.

**[NIT]** The comment `// nil body: the GitHub DELETE endpoint for reviews requires no request body.` was removed in the diff. It was a useful clarification for why nil is passed to doRequestWithBody. Consider retaining it or replacing with an equivalent inline comment.
c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
// nil body: the GitHub DELETE endpoint for reviews requires no request body.
_, err := c.doRequestWithBody(ctx, http.MethodDelete, reqURL, nil)
if err != nil {
var apiErr *APIError
1