-
Notifications
You must be signed in to change notification settings - Fork 40
Mark preëxisting lints in codebase #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Entire-Checkpoint: d12d904f72e1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Entire-Checkpoint: 4c1c73f741a4
Entire-Checkpoint: bb5d597f9fee
Entire-Checkpoint: bb5d597f9fee
Entire-Checkpoint: 68e0ff0d6706
736bdfd to
aafddf5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Marks preëxisting golangci-lint findings inline (via targeted //nolint annotations) and adjusts lint configuration so CI/local linting can run consistently without relying on “only new issues on main”.
Changes:
- Added targeted
//nolint:<linter>annotations across existing code paths/tests to acknowledge preëxisting lint violations. - Updated
.golangci.yamlto stop using merge-base/new-only filtering and to disabledupltemporarily. - Tightened one test assertion to use
errors.Isfor a sentinel error comparison.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/utils.go | Adds wrapcheck suppression on a raw read error return. |
| cmd/entire/cli/transcript_test.go | Adds goconst suppression in a transcript parsing test. |
| cmd/entire/cli/transcript.go | Adds wrapcheck suppression on os.Open error return. |
| cmd/entire/cli/strategy/strategy.go | Adds wrapcheck suppressions for preëxisting raw error returns. |
| cmd/entire/cli/strategy/registry.go | Adds ireturn suppressions for interface-returning registry functions. |
| cmd/entire/cli/strategy/messages.go | Adds unparam suppressions to keep unused params for API compatibility. |
| cmd/entire/cli/strategy/manual_commit_test.go | Adds goconst suppressions in tests. |
| cmd/entire/cli/strategy/manual_commit_rewind.go | Adds staticcheck suppression for deprecated checkpoint path helper. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Adds maintidx suppression on a complex function. |
| cmd/entire/cli/strategy/manual_commit.go | Adds ireturn suppressions on factory constructors returning interfaces. |
| cmd/entire/cli/strategy/common.go | Adds unparam suppression on createCommit signature. |
| cmd/entire/cli/strategy/cleanup.go | Adds unparam suppression on a function signature. |
| cmd/entire/cli/strategy/auto_commit.go | Adds ireturn suppression on factory constructor. |
| cmd/entire/cli/state.go | Adds nilnil/wrapcheck suppressions for preëxisting patterns. |
| cmd/entire/cli/setup_test.go | Uses errors.Is for sentinel error assertion; adds errors import. |
| cmd/entire/cli/setup.go | Adds unparam suppressions on functions with fixed params. |
| cmd/entire/cli/rewind.go | Adds maintidx/wrapcheck suppressions in rewind command flows. |
| cmd/entire/cli/paths/paths.go | Adds forbidigo suppression for os.Getwd used in RepoRoot caching. |
| cmd/entire/cli/hooks_claudecode_handlers.go | Adds maintidx suppression on a complex function. |
| cmd/entire/cli/hook_registry.go | Adds ineffassign/wastedassign suppression around redundant assignment. |
| cmd/entire/cli/config_test.go | Adds goconst suppression in config test. |
| cmd/entire/cli/config.go | Adds wrapcheck suppression for raw error return in IsEnabled. |
| cmd/entire/cli/checkpoint/temporary.go | Removes an empty line (format/lint cleanup). |
| cmd/entire/cli/agent/registry.go | Adjusts nolint list for Default agent factory behavior. |
| cmd/entire/cli/agent/claudecode/transcript.go | Adds goconst suppression in transcript parsing logic. |
| .golangci.yaml | Removes new-only merge-base filtering and disables dupl; updates ireturn allow-list entry. |
|
As for Bugbot's concerns, yep, we shouldn't ignore these issues. However, i'm purposely avoiding changing code just before our release. We can easily come back and strip these |
Description
Over in #178, we're working to make CI and local linting behave exactly the same. One of the challenges is that "gimme lint issues that aren't on
main" is a bit brittle, so we get reams of errors that were preëxisting. So i've gone and marked all of those.I've also turned off the duplicate stanza detector for now. I tried playing whackamole with adding
//nolintannotations but that didn't work well for thedupldetector, and addressing the dupes by refactoring right now isn't going to happen just as we want to release.Note
Low Risk
Primarily lint configuration and comment-only suppressions; functional behavior is effectively unchanged aside from a safer error comparison in one test.
Overview
Reduces lint noise by removing merge-base/new-issues gating in
.golangci.yaml, disablingdupl, and updatingireturnallowlists to match the current module import path.Across the CLI/strategy code, adds narrowly-scoped
//nolintannotations (e.g.,wrapcheck,goconst,maintidx,unparam,nilnil) to document and suppress pre-existing warnings without changing runtime behavior, plus a small test tweak to useerrors.Isfor the unsupported-shell sentinel.Written by Cursor Bugbot for commit abca476. This will update automatically on new commits. Configure here.