ci: enforce gofmt with golangci-lint#23
Conversation
arthursoares
left a comment
There was a problem hiding this comment.
Thanks — this is a welcome addition, and the execution is careful. Before reviewing I verified locally (with the exact pinned golangci-lint v2.12.2):
- All Go-file changes are pure whitespace:
git diff -wagainstmainis empty for every touched.gofile. - The v2 config schema is valid, and
golangci-lint rundoes enforce thegofmtformatter in v2 —make lintcomes back0 issueson this branch. golangci-lint-action@v9withinstall-only: true+make lintas the shared entrypoint is a nice pattern.
One required change, one heads-up:
Required: the generated-file exclusion isn't doing what the comment says
formatters.exclusions.generated: disable is not honored by golangci-lint run. In v2.12.2, run filters all reported issues — including formatter-produced ones — through linters.exclusions.generated, which defaults to strict; only path exclusions propagate from the formatters section. I confirmed empirically: deliberately mis-formatting itemaction_string.go (our stringer output) still yields 0 issues with this config, so the comment "Generated output must satisfy the same formatting invariant" describes an invariant the gate silently doesn't enforce. That's a misleading green check.
Fix is three lines:
linters:
default: none
exclusions:
generated: disable(Keeping the formatters.exclusions.generated: disable too is fine/harmless.)
Heads-up: interaction with #22
#22 and this PR both touch the ItemKind var block in types.go. They merge cleanly — no textual conflict — but the merged result is gofmt-dirty (the tag constants keep alignment computed before ItemKindTombstonePlain widened the column), so whichever lands second fails this very gate with no warning at merge time. We'll merge this PR first; #22 will rebase and re-run gofmt. Nothing for you to do here beyond being aware.
Also note I've retargeted the base to develop — this repo integrates there and tags releases on main.
(Review assisted by Claude Code, with an adversarial pass by Codex — which gets the credit for catching the generated-exclusion subtlety.)
Add a Makefile lint target as shared local and CI entrypoint. Configure golangci-lint with no default linters and enable only gofmt so checks can be added independently. Assisted-by: Codex Signed-off-by: Chris Henzie <chrishenzie@gmail.com>
|
Updated with fixes. I'll rebase the other PR once this merges. It looks like GitHub actions require approval to trigger here. Is there a way to approve this? Would help with verifying the action added here actually passes |
arthursoares
left a comment
There was a problem hiding this comment.
Fix verified — re-ran the falsification test with the pinned v2.12.2: a deliberately mis-formatted itemaction_string.go is now flagged (File is not properly formatted (gofmt), exit 1), where the previous config silently reported 0 issues. The gate now enforces exactly what the config comment claims.
On the Actions question: runs from first-time contributors need manual approval — I've approved yours, and both jobs came back green (first-ever run of the lint gate). Once this PR merges you'll be a prior contributor and future runs should trigger without the gate.
Add a
Makefilelint target as shared local and CI entrypoint. Configuregolangci-lintwith no default linters and enable onlygofmtso checks can be added independently.Assisted-by: Codex