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

Build broken: github/reviews.go duplicates github/review.go — redeclared types and methods #116

New Issue
Closed
opened 2026-05-13 19:07:57 +00:00 by rodin · 1 comment
rodin commented 2026-05-13 19:07:57 +00:00
Owner
Copy Link
Copy Source

What was missed

The github/ package fails to compile due to duplicate type and method declarations introduced when github/reviews.go was added alongside the existing github/review.go. Running go build ./... produces:

# gitea.weiker.me/rodin/review-bot/github
github/reviews.go:23:6: reviewResponse redeclared in this block
    github/review.go:41:6: other declaration of reviewResponse
github/reviews.go:49:6: dismissReviewRequest redeclared in this block
    github/review.go:52:6: other declaration of dismissReviewRequest
github/reviews.go:54:6: userResponse redeclared in this block
    github/identity.go:10:6: other declaration of userResponse
github/reviews.go:71:18: method Client.PostReview already declared at github/review.go:88:18
github/reviews.go:114:18: method Client.ListReviews already declared at github/review.go:141:18
github/reviews.go:160:18: method Client.DeleteReview already declared at github/review.go:172:18
github/reviews.go:171:18: method Client.DismissReview already declared at github/review.go:191:18
github/reviews.go:187:18: method Client.GetAuthenticatedUser already declared at github/identity.go:15:18
github/review.go:120:17: c.doRequestWithBody undefined (type *Client has no field or method doRequestWithBody)

github/review.go was added in PR #105 with a doRequestWithBody helper dependency that was never defined in client.go. github/reviews.go was added in PR #106 as a replacement/update, but github/review.go was not removed — leaving both files in the tree and causing the duplicate declarations.

Additionally, github/review.go references c.doRequestWithBody which does not exist on *Client.

Source

What needs to happen

  1. Delete github/review.go (its intended content has been superseded by github/reviews.go)
  2. Ensure github/reviews.go has all necessary method implementations without relying on doRequestWithBody
  3. Verify go build ./... and go test ./... pass
  4. Add CI build check to prevent future regressions

References

  • Build failure confirmed via go build ./... on main branch
## What was missed The `github/` package fails to compile due to duplicate type and method declarations introduced when `github/reviews.go` was added alongside the existing `github/review.go`. Running `go build ./...` produces: ``` # gitea.weiker.me/rodin/review-bot/github github/reviews.go:23:6: reviewResponse redeclared in this block github/review.go:41:6: other declaration of reviewResponse github/reviews.go:49:6: dismissReviewRequest redeclared in this block github/review.go:52:6: other declaration of dismissReviewRequest github/reviews.go:54:6: userResponse redeclared in this block github/identity.go:10:6: other declaration of userResponse github/reviews.go:71:18: method Client.PostReview already declared at github/review.go:88:18 github/reviews.go:114:18: method Client.ListReviews already declared at github/review.go:141:18 github/reviews.go:160:18: method Client.DeleteReview already declared at github/review.go:172:18 github/reviews.go:171:18: method Client.DismissReview already declared at github/review.go:191:18 github/reviews.go:187:18: method Client.GetAuthenticatedUser already declared at github/identity.go:15:18 github/review.go:120:17: c.doRequestWithBody undefined (type *Client has no field or method doRequestWithBody) ``` `github/review.go` was added in PR #105 with a `doRequestWithBody` helper dependency that was never defined in `client.go`. `github/reviews.go` was added in PR #106 as a replacement/update, but `github/review.go` was not removed — leaving both files in the tree and causing the duplicate declarations. Additionally, `github/review.go` references `c.doRequestWithBody` which does not exist on `*Client`. ## Source - Introduced by: PRs #105 and #106 - File: `github/review.go` (should be deleted or its methods removed) - File: `github/reviews.go` (the intended current implementation) - Method: `doRequestWithBody` — referenced in `github/review.go:120,177` but never defined ## What needs to happen 1. Delete `github/review.go` (its intended content has been superseded by `github/reviews.go`) 2. Ensure `github/reviews.go` has all necessary method implementations without relying on `doRequestWithBody` 3. Verify `go build ./...` and `go test ./...` pass 4. Add CI build check to prevent future regressions ## References - Build failure confirmed via `go build ./...` on main branch
rodin added the ai-reviewbug labels 2026-05-13 19:07:57 +00:00
rodin self-assigned this 2026-05-13 22:27:55 +00:00
rodin commented 2026-05-13 22:29:49 +00:00
Author
Owner
Copy Link
Copy Source

Plan: Fix build — remove duplicate github/review.go

Problem

github/review.go and github/reviews.go both declare the same types (reviewResponse, dismissReviewRequest) and methods (PostReview, ListReviews, DeleteReview, DismissReview). Additionally, github/identity.go declares userResponse and GetAuthenticatedUser which are also in reviews.go. The build fails with redeclaration errors.

github/review.go also references c.doRequestWithBody which does not exist on *Client.

Root Cause

PR #105 added github/review.go with a doRequestWithBody dependency. PR #106 added github/reviews.go as a replacement using doJSONRequest, but never removed review.go.

Proposed Approach

  1. Delete github/review.go — superseded by github/reviews.go
  2. Delete github/identity.go — its userResponse type and GetAuthenticatedUser are already in reviews.go
  3. Add doJSONRequest method to client.go — reviews.go calls it but it does not exist. It should marshal a payload to JSON, then perform the HTTP request with Content-Type: application/json.
  4. Update github/review_test.go — references postReviewRequest (now reviewCreateRequest), ErrCannotDeleteSubmittedReview, ErrConflictingCommitIDs, and translateGitHubReviewState. Tests that exercise deleted logic should be removed or rewritten against the reviews.go API.
  5. Fix github/helpers_test.go — calls c.SetRetryBackoff(...) expecting an error return, but the signature returns nothing. Align.
  6. Verify go build ./... and go test ./... pass

Constraints

  • Must not change the public API (interface vcs.Provider contracts stay the same)
  • reviews.go is the canonical implementation — preserve its behavior
  • Keep identity_test.go — tests cover GetAuthenticatedUser which lives in reviews.go now

Error/Edge Cases

  • ErrCannotDeleteSubmittedReview from review.go provides better error reporting for 422 on delete. Consider keeping that sentinel error and the associated check in reviews.go's DeleteReview.
  • ErrConflictingCommitIDs from review.go validates commit ID consistency across comments. The reviews.go version silently takes the first — consider adding the validation back.
  • translateGitHubReviewState from review.go maps GitHub states to canonical vcs states. The reviews.go version passes states through raw. Need to verify which is correct per the vcs interface contract.

Testing Strategy

  • go build ./... must pass
  • go test ./... must pass
  • Existing test coverage for PostReview, ListReviews, DeleteReview, DismissReview, GetAuthenticatedUser preserved

Open Questions

  • Should ErrConflictingCommitIDs validation be ported to reviews.go? The current reviews.go silently takes first CommitID. Going with: no, keep it simple — the issue is about fixing the build, not adding features.
  • Should translateGitHubReviewState be ported? reviews.go passes state through raw. Going with: check what tests expect and align.
## Plan: Fix build — remove duplicate github/review.go ### Problem `github/review.go` and `github/reviews.go` both declare the same types (`reviewResponse`, `dismissReviewRequest`) and methods (`PostReview`, `ListReviews`, `DeleteReview`, `DismissReview`). Additionally, `github/identity.go` declares `userResponse` and `GetAuthenticatedUser` which are also in `reviews.go`. The build fails with redeclaration errors. `github/review.go` also references `c.doRequestWithBody` which does not exist on `*Client`. ### Root Cause PR #105 added `github/review.go` with a `doRequestWithBody` dependency. PR #106 added `github/reviews.go` as a replacement using `doJSONRequest`, but never removed `review.go`. ### Proposed Approach 1. **Delete `github/review.go`** — superseded by `github/reviews.go` 2. **Delete `github/identity.go`** — its `userResponse` type and `GetAuthenticatedUser` are already in `reviews.go` 3. **Add `doJSONRequest` method to `client.go`** — `reviews.go` calls it but it does not exist. It should marshal a payload to JSON, then perform the HTTP request with Content-Type: application/json. 4. **Update `github/review_test.go`** — references `postReviewRequest` (now `reviewCreateRequest`), `ErrCannotDeleteSubmittedReview`, `ErrConflictingCommitIDs`, and `translateGitHubReviewState`. Tests that exercise deleted logic should be removed or rewritten against the `reviews.go` API. 5. **Fix `github/helpers_test.go`** — calls `c.SetRetryBackoff(...)` expecting an error return, but the signature returns nothing. Align. 6. **Verify `go build ./...` and `go test ./...` pass** ### Constraints - Must not change the public API (interface `vcs.Provider` contracts stay the same) - `reviews.go` is the canonical implementation — preserve its behavior - Keep `identity_test.go` — tests cover `GetAuthenticatedUser` which lives in `reviews.go` now ### Error/Edge Cases - `ErrCannotDeleteSubmittedReview` from `review.go` provides better error reporting for 422 on delete. Consider keeping that sentinel error and the associated check in `reviews.go`'s `DeleteReview`. - `ErrConflictingCommitIDs` from `review.go` validates commit ID consistency across comments. The `reviews.go` version silently takes the first — consider adding the validation back. - `translateGitHubReviewState` from `review.go` maps GitHub states to canonical vcs states. The `reviews.go` version passes states through raw. Need to verify which is correct per the `vcs` interface contract. ### Testing Strategy - `go build ./...` must pass - `go test ./...` must pass - Existing test coverage for PostReview, ListReviews, DeleteReview, DismissReview, GetAuthenticatedUser preserved ### Open Questions - Should `ErrConflictingCommitIDs` validation be ported to `reviews.go`? The current `reviews.go` silently takes first CommitID. Going with: no, keep it simple — the issue is about fixing the build, not adding features. - Should `translateGitHubReviewState` be ported? `reviews.go` passes state through raw. Going with: check what tests expect and align.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label ai-review bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees rodin
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#116
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?