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

feat(vcs): add CommitID to ReviewRequest (#115) #118

Merged
aweiker merged 5 commits from review-bot-issue-115 into feature/github-support 2026-05-14 01:49:33 +00:00
Conversation 47 Commits 5 Files Changed 4
+3 -36
3 changed files with 3 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 9a6298cc4f - Show all commits
cmd/review-bot/main.go
+1 -1
View File
@@ -439,7 +439,7 @@ func main() {
inlineComments = append(inlineComments, vcs.ReviewComment{ inlineComments = append(inlineComments, vcs.ReviewComment{
Path: f.File, Path: f.File,
Position: pos, Position: pos,
CommitID: pr.Head.SHA, CommitID: evaluatedSHA,
Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
}) })
} }
1
gitea/adapter_test.go
+1 -35
View File
@@ -214,40 +214,6 @@ func TestAdapter_PostReview_EventTranslation(t *testing.T) {
} }
sonnet-review-bot commented 2026-05-13 23:25:52 +00:00
Review
Copy Link
Copy Source

[NIT] TestAdapter_PostReview_CommitID and TestAdapter_PostReview_CommitID_Threading (added later in the file) both test largely the same behaviour — that req.CommitID is forwarded to the underlying client and echoed back in the returned review. Having two tests for the same assertion is minor duplication; one could be removed or merged.

**[NIT]** TestAdapter_PostReview_CommitID and TestAdapter_PostReview_CommitID_Threading (added later in the file) both test largely the same behaviour — that req.CommitID is forwarded to the underlying client and echoed back in the returned review. Having two tests for the same assertion is minor duplication; one could be removed or merged.
} }
func TestAdapter_PostReview_CommitID(t *testing.T) {
var gotCommitID string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
var payload struct {
CommitID string `json:"commit_id"`
}
json.NewDecoder(r.Body).Decode(&payload)
gotCommitID = payload.CommitID
json.NewEncoder(w).Encode(map[string]any{
"id": 1,
"body": "test",
"user": map[string]any{"login": "bot"},
"commit_id": payload.CommitID,
})
}))
defer server.Close()
client := gitea.NewClient(server.URL, "token")
adapter := gitea.NewAdapter(client)
_, err := adapter.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
Body: "test",
Event: vcs.ReviewEventApprove,
CommitID: "sha256abc",
})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if gotCommitID != "sha256abc" {
t.Errorf("expected commit_id %q forwarded to client, got %q", "sha256abc", gotCommitID)
}
}
func TestAdapter_PostReview_WithComments_PositionTranslation(t *testing.T) { func TestAdapter_PostReview_WithComments_PositionTranslation(t *testing.T) {
diff := `diff --git a/main.go b/main.go diff := `diff --git a/main.go b/main.go
--- a/main.go --- a/main.go
2
 @@ -443,7 +409,7 @@ func TestAdapter_RequestReviewerSelf(t *testing.T) {
} }
} }
func TestAdapter_PostReview_CommitID_Threading(t *testing.T) { func TestAdapter_PostReview_CommitID(t *testing.T) {
var gotPayload struct { var gotPayload struct {
Body string `json:"body"` Body string `json:"body"`
Event string `json:"event"` Event string `json:"event"`
vcs/types.go
+1
View File
3
 @@ -98,5 +98,6 @@ type ReviewRequest struct {
// If empty, the platform defaults to the current PR head. // If empty, the platform defaults to the current PR head.
// Adapters use this as the primary commit anchor for the review submission. // Adapters use this as the primary commit anchor for the review submission.
sonnet-review-bot commented 2026-05-13 21:30:14 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] Minor inconsistent alignment: CommitID string has extra whitespace padding (string ) before the json tag compared to the other fields. This is a gofmt style deviation that should be cleaned up.

**[NIT]** Minor inconsistent alignment: `CommitID string` has extra whitespace padding (`string `) before the json tag compared to the other fields. This is a gofmt style deviation that should be cleaned up.
CommitID string `json:"commit_id,omitempty"` CommitID string `json:"commit_id,omitempty"`
sonnet-review-bot commented 2026-05-13 23:25:52 +00:00
Outdated
Review
Copy Link
Copy Source

[NIT] Removing the blank line between CommitID and Comments fields reduces readability in what is an exported, public-API struct. The blank line was providing visual grouping between the scalar fields (Body, Event, CommitID) and the slice field (Comments). This is purely cosmetic but the change is a minor readability regression with no functional benefit.

**[NIT]** Removing the blank line between CommitID and Comments fields reduces readability in what is an exported, public-API struct. The blank line was providing visual grouping between the scalar fields (Body, Event, CommitID) and the slice field (Comments). This is purely cosmetic but the change is a minor readability regression with no functional benefit.
Comments []ReviewComment `json:"comments,omitempty"` Comments []ReviewComment `json:"comments,omitempty"`
} }
sonnet-review-bot commented 2026-05-13 21:24:40 +00:00
Review
Copy Link
Copy Source

[NIT] Minor struct field alignment inconsistency: CommitID string is followed by two spaces before json:... while neighboring fields use one tab. This is a gofmt artifact from the diff display, but worth confirming gofmt was run on the final file. The full file content shows correctly formatted alignment, so this is likely just diff rendering.

**[NIT]** Minor struct field alignment inconsistency: `CommitID string` is followed by two spaces before `json:...` while neighboring fields use one tab. This is a gofmt artifact from the diff display, but worth confirming `gofmt` was run on the final file. The full file content shows correctly formatted alignment, so this is likely just diff rendering.