Skip to content

ci: enforce gofmt with golangci-lint#23

Merged
arthursoares merged 1 commit into
arthursoares:developfrom
chrishenzie:lint/gofmt
Jul 22, 2026
Merged

ci: enforce gofmt with golangci-lint#23
arthursoares merged 1 commit into
arthursoares:developfrom
chrishenzie:lint/gofmt

Conversation

@chrishenzie

Copy link
Copy Markdown

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

@arthursoares
arthursoares changed the base branch from main to develop July 22, 2026 09:19

@arthursoares arthursoares left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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 -w against main is empty for every touched .go file.
  • The v2 config schema is valid, and golangci-lint run does enforce the gofmt formatter in v2 — make lint comes back 0 issues on this branch.
  • golangci-lint-action@v9 with install-only: true + make lint as 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>
@chrishenzie

chrishenzie commented Jul 22, 2026

Copy link
Copy Markdown
Author

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 arthursoares left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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.

@arthursoares
arthursoares merged commit e519192 into arthursoares:develop Jul 22, 2026
2 checks passed
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