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

feat(cmd): wire --provider and --base-url flags into CLI (Phase 5) #106

Merged
aweiker merged 17 commits from review-bot-issue-82 into feature/github-support 2026-05-13 17:16:28 +00:00
Conversation 98 Commits 17 Files Changed 14
-1
1 changed files with 0 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 271ea7f5fe - Show all commits
github/client.go
-1
View File
1
 @@ -384,7 +384,6 @@ func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, r
opts.extraHeaders = map[string]string{"Content-Type": "application/json"}
sonnet-review-bot commented 2026-05-13 13:49:34 +00:00
Review
Copy Link
Copy Source

[NIT] Stray blank line between the closing brace of doRequestWithBody and the start of doJSONRequest doc comment — there is also a missing blank line before the closing } of doRequestWithBody (the } is on the line immediately after return). Minor formatting inconsistency that gofmt would not catch but goimports might flag, and it reduces readability.

**[NIT]** Stray blank line between the closing brace of `doRequestWithBody` and the start of `doJSONRequest` doc comment — there is also a missing blank line before the closing `}` of `doRequestWithBody` (the `}` is on the line immediately after `return`). Minor formatting inconsistency that `gofmt` would not catch but goimports might flag, and it reduces readability.
}
return c.doRequestCore(ctx, method, reqURL, opts)
}
gpt-review-bot commented 2026-05-13 13:51:45 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] Minor formatting: there is an extra blank line between the closing brace of doRequestWithBody and the start of doJSONRequest that could be removed for consistency.

**[NIT]** Minor formatting: there is an extra blank line between the closing brace of doRequestWithBody and the start of doJSONRequest that could be removed for consistency.
sonnet-review-bot commented 2026-05-13 14:15:32 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] There is a spurious blank line before the closing brace of doRequestWithBody (after return c.doRequestCore(...)). Minor formatting issue that gofmt would not flag since it's inside a function body, but it's visually inconsistent.

**[NIT]** There is a spurious blank line before the closing brace of `doRequestWithBody` (after `return c.doRequestCore(...)`). Minor formatting issue that gofmt would not flag since it's inside a function body, but it's visually inconsistent.
sonnet-review-bot commented 2026-05-13 15:51:02 +00:00
Review
Copy Link
Copy Source

[NIT] doJSONRequest is added but not yet called by any public methods in the diff (the github package has no PostReview/DismissReview/etc. visible in the diff). This is fine for an incremental PR, but it means the new helper is untested at the integration level. The unit tests for doJSONRequest itself are present and cover the retry path.

**[NIT]** doJSONRequest is added but not yet called by any public methods in the diff (the github package has no PostReview/DismissReview/etc. visible in the diff). This is fine for an incremental PR, but it means the new helper is untested at the integration level. The unit tests for doJSONRequest itself are present and cover the retry path.
// doJSONRequest performs an HTTP request with a JSON body and returns the response body.
3