Skip to content

Conversation

@satococoa
Copy link
Owner

@satococoa satococoa commented Nov 17, 2025

Summary

  • migrate golangci-lint tooling to v2 and configure formatters
  • enable errorlint in the lint suite
  • fix uncovered errorlint issues (wrapping fmt.Errorf, using errors.As, etc.)

Testing

  • go tool golangci-lint run
  • go tool task test

Summary by CodeRabbit

  • Chores
    • Standardized lint/format configuration into per-section settings and switched formatting to golangci-lint (gofmt+goimports via golangci); updated CI lint action and dependency/tool versions.
  • Bug Fixes
    • Improved error wrapping and more robust error handling to aid diagnostics; tightened resource-close handling to avoid ignored errors.
  • Documentation
    • Updated formatting instructions to reflect the new golangci-lint formatter.
  • Tests
    • Adjusted tests to report close errors more accurately.

Copilot AI review requested due to automatic review settings November 17, 2025 14:21
@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Replaces .golangci.yml structure with per-linter settings and formatters, switches formatting to golangci-lint fmt, upgrades many Go dependencies, standardizes error wrapping with %w and errors.As, and makes small test, CI, and resource-close handling adjustments.

Changes

Cohort / File(s) Summary
Linting & Formatting config
\.golangci\.yml, Taskfile\.yml, docs/architecture.md, AGENTS.md
Replaces deprecated linters-settings with linters/settings, linters/exclusions, formatters, and issues sections; reorganizes enabled linters and settings (depguard, errcheck, gocritic, govet, gosec, revive, etc.); adds exclusions (tests, lll/go:generate); enables gofmt and goimports via golangci formatter and sets goimports local prefixes; updates format command to golangci-lint fmt.
Dependency updates
go.mod
Upgrades many primary and indirect dependencies (e.g., testify v1.10.0→v1.11.1, gopkg.in/yaml.v3 v3.0.3→v3.0.4, golang.org/x/term v0.32.0→v0.36.0); adds codeberg.org/chavacava/garif v0.2.0; broad transitive churn and tooling bumps including golangci-lint v2.x.
Error handling & resource safety
cmd/wtp/add.go, cmd/wtp/remove.go, internal/hooks/executor.go, internal/git/repository.go
Replaces several fmt.Errorf usages to wrap errors with %w; uses errors.As to inspect git command exit errors; wraps cmd.Wait() errors with %w; defers file Close() via ignored-close anonymous functions in some cases.
Tests & small control-flow edits
cmd/wtp/completion_config_test.go, internal/git/repository_integration_test.go
Changes deferred Close() to ignore close errors and fixes test failure message to use the actual close error variable; replaces worktree assignment loop with a switch for branch selection.
CI workflow
.github/workflows/ci.yml
Updates GitHub Action to golangci/golangci-lint-action@v7 and pins golangci-lint to v2.6.2.

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

  • Areas needing extra attention:
    • .golangci.yml — verify new per-linter settings, exclusions, and formatter local-prefixes are correct and equivalent to previous behavior.
    • go.mod — review dependency upgrades for compatibility and potential breakage from transitive changes.
    • Error-wrapping changes (%w, errors.As) — ensure callers/tests that previously matched errors still behave correctly.
    • Taskfile/docs/CI — confirm formatting and CI lint action behave as intended with golangci-lint v2.x.
    • Places where Close() errors are ignored — ensure they don't mask real resource issues during tests or runtime.

Poem

🐰 I hopped through diffs with careful paws,
Wrapped every error with tidy laws.
Linters changed their morning song,
Formatters hum as they move along.
Dependencies climbed a leafy track — hooray, hop on!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: migrate golangci-lint v2' clearly and concisely summarizes the main objective of the changeset: migrating from golangci-lint v1 to v2. All major changes across files (.golangci.yml, Taskfile.yml, go.mod, .github/workflows/ci.yml) and the error handling fixes align with this migration goal.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch migrate-golangci

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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)
Copy link

Copilot AI Nov 17, 2025

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.

Copilot uses AI. Check for mistakes.
Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between be2e926 and 8e51a77.

📒 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: none and an explicit enable list. The errorlint linter has been added as specified in the PR objectives, and the existing linters have been appropriately retained. The inclusion of copyloopvar is 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:generate directives. 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-prefixes value (github.com/satococoa/wtp/v2) matches the actual module name declared in go.mod to ensure consistent import grouping.

Comment on lines +38 to 90
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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.

@satococoa satococoa merged commit 9a8bf4a into main Nov 22, 2025
7 checks passed
@satococoa satococoa deleted the migrate-golangci branch November 22, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants