Skip to content

Migrate to golangci-lint v2 and optimize CI/CD#51

Open
odysseus0 wants to merge 1 commit intomainfrom
golangci-v2-migration
Open

Migrate to golangci-lint v2 and optimize CI/CD#51
odysseus0 wants to merge 1 commit intomainfrom
golangci-v2-migration

Conversation

@odysseus0
Copy link

Summary

  • Migrates golangci-lint configuration to v2 schema
  • Optimizes CI by using golangci-lint-action@v8 (eliminates 93s compilation time)
  • Implements tools.go pattern for version consistency between local and CI
  • Fixes gosec warnings in health check handlers

Changes

Config Migration:

  • .golangci.yaml: Updated to v2 schema (default: all, nested linters.settings)
  • Disabled noisy linters: funcorder, noinlineerr, wsl_v5, complexity metrics
  • Kept tagliatelle enabled (new projects should enforce snake_case JSON tags)

CI Optimization:

  • .github/workflows/checks.yml: Use golangci-lint-action@v8 with pre-built binaries
  • Extract golangci-lint version from go.mod for local/CI parity
  • Remove manual make install-tools step (action handles it)

Tool Management:

  • tools/tools.go: New file with blank imports (single source of truth for tool versions)
  • Makefile: Simplified to use go.mod versions (removed @Version suffixes)
  • Removed staticcheck (bundled in golangci-lint), added gci for import formatting

Bug Fixes:

  • httpserver/handler.go: Explicitly handle w.Write() errors to satisfy gosec

Test Plan

  • make install-tools - Tools install correctly from go.mod
  • make lint - 0 issues locally
  • make test - All tests pass
  • CI will verify golangci-lint-action works correctly

Learnings Applied

All changes are based on optimizations discovered while migrating protect-rpc from golangci-lint v2.1.2 to v2.5.0, where we:

  1. Discovered the tools.go pattern for version consistency
  2. Found golangci-lint-action eliminates 93s build time
  3. Learned v2 schema enforcement and linter configurations

🤖 Generated with Claude Code

Apply all learnings from protect-rpc optimization work:

**Config changes:**
- Migrate .golangci.yaml to v2 schema (default: all, nested settings)
- Disable noisy linters: funcorder, noinlineerr, wsl_v5, complexity metrics
- Keep tagliatelle enabled (new projects should use snake_case JSON)

**CI optimization:**
- Use golangci-lint-action@v8 (pre-built binaries, no compilation)
- Extract version from go.mod for local/CI consistency
- Simplify workflow by removing manual tool installation

**Tool management:**
- Add tools/tools.go with blank imports (single source of truth)
- Remove staticcheck (bundled in golangci-lint)
- Add gci for import formatting
- Simplify Makefile to use go.mod versions

**Bug fixes:**
- Fix gosec warnings: explicitly handle w.Write() errors

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@flashbots flashbots deleted a comment from claude bot Jan 20, 2026
@flashbots flashbots deleted a comment from claude bot Jan 20, 2026
@metachris
Copy link
Collaborator

@claude please review this PR

@claude
Copy link
Contributor

claude bot commented Jan 20, 2026

Claude finished @metachris's task in 1m 16s —— View job


PR Review: golangci-lint v2 Migration

Overall this is a well-structured migration to golangci-lint v2 with good CI optimizations. The changes follow best practices for tool version management. Below are my detailed findings.


✅ What Looks Good

1. CI Workflow Optimization (.github/workflows/checks.yml)

  • Using golangci-lint-action@v8 with pre-built binaries is a great optimization
  • Extracting version from go.mod ensures local/CI parity - nice pattern
  • Simplified workflow by removing manual make install-tools

2. tools.go Pattern (tools/tools.go)

  • Proper use of //go:build tools constraint
  • Blank imports create single source of truth for tool versions
  • Well-documented with comments explaining each tool's purpose

3. Makefile Improvements

  • Clean extraction of tool paths and versions using go list -m
  • GOBIN := $(CURDIR)/bin with export GOBIN is the right pattern for project-local tools
  • Good separation of concerns with dedicated install-tools target

4. gosec Fix (httpserver/handler.go)

  • Using _, _ = w.Write(...) with comments explaining intentional error ignoring is the correct approach
  • Comments are clear and justify the decision

5. .gitignore Update

  • Adding /bin for project-local tool binaries is correct

⚠️ Suggestions & Questions

1. Makefile - Version extraction function is defined but not used

The modver function is defined at line 12-14 but never called:

define modver
$(shell go list -m -f '{{.Version}}' $(1))
endef

The tool installation targets use go install $(GOLANGCI_LINT) without versions, relying on go.mod for resolution. This is fine, but the unused modver macro could be removed if not needed.

2. golangci.yaml - Disabled linters worth noting

The config disables both wsl and wsl_v5. Since v2 likely uses only one of these, you might be able to remove the redundant entry. Also, the exhaustruct settings at lines 67-92 exist but exhaustruct is disabled at line 29 - these settings have no effect.

3. CI Workflow - Potential race condition

At .github/workflows/checks.yml:39, the version extraction:

run: echo "version=$(go list -m -f '{{.Version}}' github.com/golangci/golangci-lint/v2)" >> $GITHUB_OUTPUT

This requires go.mod to be present and go to be installed first. The ordering looks correct (setup-go comes before this step), but worth verifying this works on a fresh runner.

4. go.mod bloat

The go.mod file went from ~15 direct dependencies to 230 lines. This is expected when using tools.go (since all tool dependencies become indirect dependencies), but it does increase the maintenance burden. This is a standard trade-off for the tools.go pattern.


🔍 Minor Observations

  1. Makefile line 70: $(GOBIN)/golangci-lint run - uses local binary, which is correct after install-tools

  2. Makefile line 74-76: The fmt target uses gofmt -s -w . directly (system-installed) but $(GOBIN)/gci and $(GOBIN)/gofumpt (local). Consider using $(GOBIN)/gofmt or go fmt ./... for consistency, or document that gofmt is expected to be system-installed.

  3. tools/tools.go imports: The import path for golangci-lint uses /v2/cmd/golangci-lint which is correct for the CLI binary


✅ Summary

Recommendation: Approve with minor suggestions

This is a solid migration that:

  • Properly implements the tools.go pattern for version consistency
  • Optimizes CI with pre-built golangci-lint binaries
  • Correctly updates to v2 schema with sensible linter choices
  • Fixes gosec warnings appropriately

The suggestions above are minor improvements, not blockers. The core approach is sound and follows Go tooling best practices.


@metachris metachris force-pushed the main branch 4 times, most recently from 0536ea0 to 27263ef Compare January 22, 2026 16:23
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