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.
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
Delete github/review.go (its intended content has been superseded by github/reviews.go)
Ensure github/reviews.go has all necessary method implementations without relying on doRequestWithBody
Verify go build ./... and go test ./... pass
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
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
Delete github/review.go — superseded by github/reviews.go
Delete github/identity.go — its userResponse type and GetAuthenticatedUser are already in reviews.go
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.
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.
Fix github/helpers_test.go — calls c.SetRetryBackoff(...) expecting an error return, but the signature returns nothing. Align.
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.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What was missed
The
github/package fails to compile due to duplicate type and method declarations introduced whengithub/reviews.gowas added alongside the existinggithub/review.go. Runninggo build ./...produces:github/review.gowas added in PR #105 with adoRequestWithBodyhelper dependency that was never defined inclient.go.github/reviews.gowas added in PR #106 as a replacement/update, butgithub/review.gowas not removed — leaving both files in the tree and causing the duplicate declarations.Additionally,
github/review.goreferencesc.doRequestWithBodywhich does not exist on*Client.Source
github/review.go(should be deleted or its methods removed)github/reviews.go(the intended current implementation)doRequestWithBody— referenced ingithub/review.go:120,177but never definedWhat needs to happen
github/review.go(its intended content has been superseded bygithub/reviews.go)github/reviews.gohas all necessary method implementations without relying ondoRequestWithBodygo build ./...andgo test ./...passReferences
go build ./...on main branchPlan: Fix build — remove duplicate github/review.go
Problem
github/review.goandgithub/reviews.goboth declare the same types (reviewResponse,dismissReviewRequest) and methods (PostReview,ListReviews,DeleteReview,DismissReview). Additionally,github/identity.godeclaresuserResponseandGetAuthenticatedUserwhich are also inreviews.go. The build fails with redeclaration errors.github/review.goalso referencesc.doRequestWithBodywhich does not exist on*Client.Root Cause
PR #105 added
github/review.gowith adoRequestWithBodydependency. PR #106 addedgithub/reviews.goas a replacement usingdoJSONRequest, but never removedreview.go.Proposed Approach
github/review.go— superseded bygithub/reviews.gogithub/identity.go— itsuserResponsetype andGetAuthenticatedUserare already inreviews.godoJSONRequestmethod toclient.go—reviews.gocalls it but it does not exist. It should marshal a payload to JSON, then perform the HTTP request with Content-Type: application/json.github/review_test.go— referencespostReviewRequest(nowreviewCreateRequest),ErrCannotDeleteSubmittedReview,ErrConflictingCommitIDs, andtranslateGitHubReviewState. Tests that exercise deleted logic should be removed or rewritten against thereviews.goAPI.github/helpers_test.go— callsc.SetRetryBackoff(...)expecting an error return, but the signature returns nothing. Align.go build ./...andgo test ./...passConstraints
vcs.Providercontracts stay the same)reviews.gois the canonical implementation — preserve its behavioridentity_test.go— tests coverGetAuthenticatedUserwhich lives inreviews.gonowError/Edge Cases
ErrCannotDeleteSubmittedReviewfromreview.goprovides better error reporting for 422 on delete. Consider keeping that sentinel error and the associated check inreviews.go'sDeleteReview.ErrConflictingCommitIDsfromreview.govalidates commit ID consistency across comments. Thereviews.goversion silently takes the first — consider adding the validation back.translateGitHubReviewStatefromreview.gomaps GitHub states to canonical vcs states. Thereviews.goversion passes states through raw. Need to verify which is correct per thevcsinterface contract.Testing Strategy
go build ./...must passgo test ./...must passOpen Questions
ErrConflictingCommitIDsvalidation be ported toreviews.go? The currentreviews.gosilently takes first CommitID. Going with: no, keep it simple — the issue is about fixing the build, not adding features.translateGitHubReviewStatebe ported?reviews.gopasses state through raw. Going with: check what tests expect and align.