-
Notifications
You must be signed in to change notification settings - Fork 10
chore: migrate golangci-lint v2 #47
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
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces Changes
Sequence Diagram(s)(omitted — changes are configuration, dependency updates, and localized error/close handling; no new high-level runtime control flow requiring a sequence diagram) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
This PR upgrades golangci-lint to v2 and enhances code quality by enabling errorlint checks and fixing related issues.
Key changes:
- Upgraded tooling: Migrated from golangci-lint v1 to v2 with updated configuration format
- Enabled errorlint: Added error wrapping validation to catch improper error handling
- Fixed error handling: Updated fmt.Errorf calls to use %w verb and improved defer error handling patterns
- Updated dependencies: Bumped all golangci-lint plugin and dependency versions
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .golangci.yml | Migrated to v2 config format with formatters section and errorlint enabled |
| go.mod | Updated golangci-lint to v2 module path and bumped dependencies |
| go.sum | Updated checksums for new dependency versions |
| internal/hooks/executor.go | Fixed error wrapping and defer error handling |
| internal/git/repository.go | Replaced type assertion with errors.As for proper error checking |
| internal/git/repository_integration_test.go | Refactored if-else chain to switch statement |
| cmd/wtp/remove.go | Fixed error wrapping in error formatting |
| cmd/wtp/add.go | Fixed error wrapping in multi-line error messages |
| cmd/wtp/completion_config_test.go | Fixed variable shadowing and added defer error handling |
| Taskfile.yml | Updated fmt task to use golangci-lint fmt |
| docs/architecture.md | Updated documentation for new golangci-lint v2 usage |
| AGENTS.md | Updated format command documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runErr := app.Run(context.Background(), args) | ||
| if closeErr := w.Close(); closeErr != nil { | ||
| t.Fatalf("failed to close writer: %v", err) | ||
| t.Fatalf("failed to close writer: %v", closeErr) |
Copilot
AI
Nov 17, 2025
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.
The error verb should be %w instead of %v to maintain proper error wrapping, which is the pattern being enforced throughout this PR.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.golangci.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test (macos-latest, 1.24)
🔇 Additional comments (2)
.golangci.yml (2)
1-36: v2 migration structure looks good—linters properly configured.The v2 schema is correctly applied with
default: noneand an explicitenablelist. Theerrorlintlinter has been added as specified in the PR objectives, and the existing linters have been appropriately retained. The inclusion ofcopyloopvaris good for Go 1.22+ compatibility.
91-113: Exclusions and formatters properly configured for v2.The exclusions are sensibly configured to skip mnd/goconst/gosec in test files and to skip lll checks on
//go:generatedirectives. The formatters section correctly enables gofmt and goimports as specified in the PR objectives, with goimports local-prefixes set to match the internal module path.Confirm that the goimports
local-prefixesvalue (github.com/satococoa/wtp/v2) matches the actual module name declared ingo.modto ensure consistent import grouping.
| settings: | ||
| depguard: | ||
| rules: | ||
| main: | ||
| files: | ||
| - "$all" | ||
| - "!$test" | ||
| list-mode: lax | ||
| allow: | ||
| - $gostd | ||
| - github.com/satococoa/wtp/v2 | ||
| - github.com/urfave/cli/v3 | ||
| - go.yaml.in/yaml/v3 | ||
| errcheck: | ||
| exclude-functions: | ||
| - fmt.Fprint | ||
| - fmt.Fprintf | ||
| - fmt.Fprintln | ||
| - fmt.Print | ||
| - fmt.Printf | ||
| - fmt.Println | ||
| - os.Setenv | ||
| - os.Unsetenv | ||
| govet: | ||
| enable-all: true | ||
| disable: | ||
| - fieldalignment | ||
| gocyclo: | ||
| min-complexity: 15 | ||
| misspell: | ||
| locale: US | ||
| lll: | ||
| line-length: 120 | ||
| gocritic: | ||
| enabled-tags: | ||
| - diagnostic | ||
| - experimental | ||
| - opinionated | ||
| - performance | ||
| - style | ||
| disabled-checks: | ||
| - dupImport | ||
| - ifElseChain | ||
| - octalLiteral | ||
| - whyNoLint | ||
| gosec: | ||
| excludes: | ||
| - G204 | ||
| - G304 | ||
| revive: | ||
| rules: | ||
| - name: indent-error-flow | ||
|
|
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.
Fix the YAML import path typo in depguard allow list.
Line 50 lists go.yaml.in/yaml/v3, which should be gopkg.in/yaml/v3. This appears to be a typo—the canonical import path for the YAML library is gopkg.in/yaml, not go.yaml.in/yaml.
allow:
- $gostd
- github.com/satococoa/wtp/v2
- github.com/urfave/cli/v3
- - go.yaml.in/yaml/v3
+ - gopkg.in/yaml/v3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| settings: | |
| depguard: | |
| rules: | |
| main: | |
| files: | |
| - "$all" | |
| - "!$test" | |
| list-mode: lax | |
| allow: | |
| - $gostd | |
| - github.com/satococoa/wtp/v2 | |
| - github.com/urfave/cli/v3 | |
| - go.yaml.in/yaml/v3 | |
| errcheck: | |
| exclude-functions: | |
| - fmt.Fprint | |
| - fmt.Fprintf | |
| - fmt.Fprintln | |
| - fmt.Print | |
| - fmt.Printf | |
| - fmt.Println | |
| - os.Setenv | |
| - os.Unsetenv | |
| govet: | |
| enable-all: true | |
| disable: | |
| - fieldalignment | |
| gocyclo: | |
| min-complexity: 15 | |
| misspell: | |
| locale: US | |
| lll: | |
| line-length: 120 | |
| gocritic: | |
| enabled-tags: | |
| - diagnostic | |
| - experimental | |
| - opinionated | |
| - performance | |
| - style | |
| disabled-checks: | |
| - dupImport | |
| - ifElseChain | |
| - octalLiteral | |
| - whyNoLint | |
| gosec: | |
| excludes: | |
| - G204 | |
| - G304 | |
| revive: | |
| rules: | |
| - name: indent-error-flow | |
| settings: | |
| depguard: | |
| rules: | |
| main: | |
| files: | |
| - "$all" | |
| - "!$test" | |
| list-mode: lax | |
| allow: | |
| - $gostd | |
| - github.com/satococoa/wtp/v2 | |
| - github.com/urfave/cli/v3 | |
| - gopkg.in/yaml/v3 | |
| errcheck: | |
| exclude-functions: | |
| - fmt.Fprint | |
| - fmt.Fprintf | |
| - fmt.Fprintln | |
| - fmt.Print | |
| - fmt.Printf | |
| - fmt.Println | |
| - os.Setenv | |
| - os.Unsetenv | |
| govet: | |
| enable-all: true | |
| disable: | |
| - fieldalignment | |
| gocyclo: | |
| min-complexity: 15 | |
| misspell: | |
| locale: US | |
| lll: | |
| line-length: 120 | |
| gocritic: | |
| enabled-tags: | |
| - diagnostic | |
| - experimental | |
| - opinionated | |
| - performance | |
| - style | |
| disabled-checks: | |
| - dupImport | |
| - ifElseChain | |
| - octalLiteral | |
| - whyNoLint | |
| gosec: | |
| excludes: | |
| - G204 | |
| - G304 | |
| revive: | |
| rules: | |
| - name: indent-error-flow |
🤖 Prompt for AI Agents
In .golangci.yml around lines 38 to 90, the depguard allow list contains a typo:
"go.yaml.in/yaml/v3" should be corrected to the canonical import path
"gopkg.in/yaml.v3"; update that entry in the allow list to "gopkg.in/yaml.v3" to
fix the import path.
Summary
Testing
Summary by CodeRabbit