Skip to content

ci(lint): add diff-only golangci-lint via reviewdog#126

Open
mateeullahmalik wants to merge 6 commits intomasterfrom
ci/golangci-lint-reviewdog
Open

ci(lint): add diff-only golangci-lint via reviewdog#126
mateeullahmalik wants to merge 6 commits intomasterfrom
ci/golangci-lint-reviewdog

Conversation

@mateeullahmalik
Copy link
Copy Markdown
Contributor

Summary

Adds diff-only golangci-lint to PRs via reviewdog. Only NEW lint introduced by the PR's changed lines is reported as inline review comments; the legacy baseline never blocks unrelated PRs.

Pairs with #125 (which clears the BRIDGE-tracked baseline to zero). After both merge, NEW lint that touches x/action/v1, x/supernode/v1, x/audit/v1 becomes a hard block on master.

Files

Path Purpose
.golangci.yml Pinned linter config — errcheck, govet, ineffassign, staticcheck, unused, typecheck. Excludes *.pb.go and the SDK-required HasInvariants / InvariantRegistry SA1019 noise in x/.../v1/module/module.go. Both CI and local devs converge on this rule set.
.github/workflows/lint.yml pull_request workflow running reviewdog/action-golangci-lint@v2 with filter_mode: added, reporter: github-pr-review, fail_level: error.

Behavior

  • On every PR: reviewdog runs golangci-lint v1.64.8 against the PR head, computes the diff vs master, and posts inline review comments only on lines the PR added or changed.
  • fail_level: error → merge is blocked when a new error-level finding lands inside the diff. Pre-existing legacy lint outside the diff is invisible.
  • No third-party account, no billing. Reviewdog is free for open-source repos and uses the standard GITHUB_TOKEN. Required permissions are declared in the workflow (pull-requests: write, checks: write).

Local equivalent

# Diff vs master (matches CI semantics):
golangci-lint run --config=.golangci.yml --new-from-rev=origin/master ./...

# Or full scan of BRIDGE modules (matches the gate report):
golangci-lint run --config=.golangci.yml ./x/action/v1/... ./x/supernode/v1/... ./x/audit/v1/...

Devs who want it in their editor: any golangci-lint LSP integration (gopls, nvim-lspconfig, VS Code Go) auto-picks .golangci.yml.

Why diff-only

A blanket required golangci-lint run ./... on day one would block every PR until the full repo is cleaned. Diff-only avoids that — legacy noise stays invisible until a PR touches that line, at which point it becomes the toucher's problem to fix or //nolint with a reason. Allows incremental cleanup with zero velocity loss.

Rollout plan after this lands

  1. Merge chore(lint): clear scoped golangci-lint + buf BRIDGE-surface findings #125 (clears BRIDGE-module baseline to 0).
  2. Merge this PR. Diff-only enforcement starts immediately.
  3. Watch for ~2 weeks. If false-positive rate is acceptable, expand by:
    • Adding more linters (e.g. gosimple, gocritic, revive) one-at-a-time.
    • Tightening staticcheck checks list.
  4. Optional later: add a separate weekly full-repo lint job that posts a Datadog metric of total diagnostic count, aiming for monotonic decrease.

Risks

  • Minimal. Diff-only mode means the workflow cannot regress merging on legacy noise.
  • Reviewdog is well-maintained (used by Kubernetes, Prometheus, gRPC, ~5k stars). v2 is the current major; we pin the minor by default but accept the v2 rolling tag for security patches — if you'd rather pin a SHA, say so and I'll update.
  • Worst case if the action breaks: lint job fails, but test.yml and other required checks still gate merge. Removing or disabling the workflow is a one-line change.

Verification

I ran the same config locally against the current master baseline (pre-#125): produces the same 25 findings the gate report listed, exit 0 (because fail_level only triggers on diff-introduced issues). After #125 merges, baseline goes to 0.

Note on cost

Reviewdog and golangci-lint are both free for any GitHub repo, public or private. The only resource cost is GitHub Actions minutes — public repos get unlimited free minutes; private orgs pay the standard Actions runner rate. Job runtime on this repo is ~2 minutes.

Adds two files:

1. .golangci.yml — pinned linter config (errcheck, govet, ineffassign,
   staticcheck, unused, typecheck). Conservative on purpose: only
   correctness linters today; style/complexity layered later.

   Excludes:
   - x/.../v1/module/module.go: SA1019 on HasInvariants/InvariantRegistry
     (Cosmos SDK v0.50 still requires them; deprecation gated on x/crisis
     removal).
   - Generated *.pb.go / *.pb.gw.go.

2. .github/workflows/lint.yml — pull_request workflow that runs
   reviewdog/action-golangci-lint with:
     filter_mode: added       (only NEW lines in the PR diff)
     reporter:    github-pr-review  (inline comments on the diff)
     fail_level:  error       (blocks merge on new error-level findings)

Diff-only enforcement means pre-existing baseline diagnostics never
block PRs on legacy noise — only lint introduced by the current PR is
flagged. Once a file is touched, its findings on changed lines become
the toucher's responsibility.

Reviewdog is free for open-source repos. No external account, billing,
or token setup needed: it uses the workflow-provided GITHUB_TOKEN and
the standard pull-requests:write permission.

Local equivalent (matches CI rule set):
  golangci-lint run --config=.golangci.yml \
      --new-from-rev=origin/master ./...

Pinned versions:
- golangci-lint v1.64.8
- reviewdog action @v2
- Go toolchain via existing .github/actions/setup-go (reads go.mod)
golangci-lint v1.64.8 is compiled against Go 1.24. Setting `go: "1.25"`
in the config makes it refuse to load with:

  can't load config: the Go language version (go1.24) used to build
  golangci-lint is lower than the targeted Go version (1.25)

Drop the pin and let golangci-lint use its own build version. Go 1.24 →
1.25 is syntax-compatible for our codebase, so the analyzers still parse
correctly. Re-pin to 1.25 once we move to golangci-lint v2.x (built with
Go 1.25+) — that's a separate config-schema migration, out of scope for
this PR.

Reproducer: with the old config,
  golangci-lint run --config=.golangci.yml ./...
exited 3 with the version-mismatch error before any file was checked.
With the pin removed, it loads and lints normally.
The previous attempt failed CI with:

  Error: can't load config: the Go language version (go1.24) used to
  build golangci-lint is lower than the targeted Go version (1.25.9)

Root cause: golangci-lint v1.64.8 (the latest v1.x release) is itself
compiled against Go 1.24. When it loads a module whose go.mod declares
`go 1.25.9` (Lumera's case), it refuses with the version-mismatch
error above. Removing the `go:` pin from .golangci.yml didn't help
because v1 reads the version from go.mod when the config doesn't
override it.

The fix is to bump to golangci-lint v2.x, which is built with Go 1.25+.
v2 ships a breaking config-schema change, so this commit also rewrites
.golangci.yml to the v2 schema:

- Add `version: "2"` at top level.
- Move `linters.disable-all: true` -> `linters.default: none`.
- Move `linters-settings` -> `linters.settings`.
- Move `issues.exclude-rules` -> `linters.exclusions.rules`.
- Drop `exclude-use-default: false` (replaced by `exclusions.generated: lax`).

Same rule set, same exclusions, same effective behavior. Pinned to
v2.0.2 (first stable v2.x release).

Verified locally: the new config loads cleanly under v2.0.2 and reports
the expected baseline findings on master (which #125 cleans).

No change to the workflow's diff-only semantics: still
filter_mode=added, reporter=github-pr-review, fail_level=error.
The @v2 major tag still resolves to a release that downloads
golangci-lint v1.x by default, ignoring our `golangci_lint_version: v2.0.2`
input. Result: same go1.24 vs 1.25.9 version-mismatch error as before.

Per reviewdog/action-golangci-lint release notes, golangci-lint v2
support was added in PR #779 -> v2.8.0 ("fix: migrate to golangci-lint
v2"). Pin to v2.10.0 (latest stable) so the requested v2.0.2 of
golangci-lint is actually downloaded.

Verified locally: golangci-lint v2.0.2 built with go1.25.9 loads our
v2-schema config and lints the chain repo without the version error.
v2.0.2 was itself built with Go 1.24 — same root cause as v1.64.8 but a
release later. golangci-lint refuses any config whose targeted Go
version is higher than the binary's own build version, regardless of
whether that target comes from go.mod or .golangci.yml.

v2.11.4 is built with Go 1.26.1 (>= our chain's Go 1.25.9), so the
version guard passes.

Verified locally: config loads, linter runs, exits 0 with the expected
master-baseline findings (which #125 cleans).
Copy link
Copy Markdown
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

Adds diff-only golangci-lint enforcement on pull requests using reviewdog so that only newly introduced lint findings are surfaced (and can block merges) without being affected by legacy baseline noise.

Changes:

  • Add a golangci-lint v2 config (.golangci.yml) enabling a small, correctness-focused linter set with targeted exclusions.
  • Add a PR workflow (lint.yml) that runs reviewdog/action-golangci-lint in diff-only mode and posts inline PR review comments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
.golangci.yml Introduces the golangci-lint v2 configuration, enabled linters, and exclusions for generated/proto and specific Cosmos SDK deprecation noise.
.github/workflows/lint.yml Adds a PR-only lint workflow that runs golangci-lint via reviewdog with diff-only filtering and configured failure behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .golangci.yml
Comment on lines +25 to +32
linters:
default: none
enable:
- errcheck # unchecked errors
- govet # suspicious constructs
- ineffassign # ineffectual assignments
- staticcheck # SA* + simple/style checks
- unused # dead code (funcs, consts, types, vars)
Comment thread .golangci.yml
Comment on lines +8 to +16
# via reviewdog `filter_mode: added`. Locally:
#
# golangci-lint run ./x/action/v1/... ./x/supernode/v1/... ./x/audit/v1/...
# golangci-lint run --new-from-rev=origin/master ./...
#
# Note on Go version: golangci-lint v1.64.8 is built with Go 1.24 and
# refuses modules declaring `go 1.25` in go.mod. The Lumera chain is on
# Go 1.25.9, so we must use golangci-lint v2.x (built with Go 1.25+).
# v2 ships a breaking config-schema change — this file is the v2 schema.
filter_mode: added
reporter: github-pr-review
fail_level: error
level: warning
Comment on lines +11 to +15
permissions:
contents: read
pull-requests: write # reviewdog needs this to post inline review comments
checks: write # for the check-run summary

Comment on lines +45 to +51
# golangci-lint v2.11.4 is built with Go 1.26.1; required because
# the chain repo's go.mod declares `go 1.25.9` and golangci-lint
# refuses to load configs whose targeted Go version is higher
# than the one it was built with. v2.0.2 was built with Go 1.24.
# Config in .golangci.yml uses the v2 schema.
golangci_lint_version: v2.11.4
golangci_lint_flags: "--config=.golangci.yml --timeout=5m"
Adds a parallel govulncheck job to .github/workflows/lint.yml so PRs
are scanned for Go module vulnerabilities (osv.dev) on every push.

- Uses golang/govulncheck-action@v1.0.4 (latest stable).
- Tracks the same Go toolchain as the rest of CI via go-version-file:
  go.mod (currently go 1.25.9).
- Scans the full module (./...). The action exits non-zero on any
  finding that reaches called code, which fails the job and blocks the
  PR — same fail-fast posture as the golangci-lint job above.
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