fix: name the revive rules instead of enabling all of them - #481
Merged
Conversation
The file said version 2 and then held linters-settings at the top level, which is the v1 layout. golangci-lint drops keys it does not recognise without complaining, so enable-all-rules never applied and most of revive has been switched off for as long as that line has been there. Turning it on properly reported 1,363 findings across these repositories. Almost all of it was naming constants and splitting functions to satisfy a threshold — line-length-limit, cognitive-complexity, cyclomatic, function-length and add-constant each report on a number somebody picked rather than on a defect. So the rules are named individually now, and the list is the conventions Go itself has an opinion about. Against this repository's current code that reports zero, which is the point: nothing to schedule, and the next mistake gets caught. Go vendored inside a JavaScript dependency tree is excluded — flatted ships a Go port beside its JS, is not tracked in git, and was the only thing govet reported anywhere. ineffassign joins the list, and the max-issues and uniq-by-line settings stop the output being truncated at 50 findings per linter and one finding per line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
The template test asserts the config does not switch on the whole rule set. The comment explaining why said so in those words, and the test matched its own explanation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
Twenty-five of seventy-six lines were comment, and most of it explained history rather than the file. The paragraph about v1 keys under a v2 version described a bug this file no longer has, and `config verify` catches it anyway. What stays is the three things reading the file will not tell you: that it is shared and must be changed everywhere at once, that the rule list is deliberate rather than partial, and that golangci-lint truncates its own output by default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
Someone reading nats-client's config does not care that a swamp extension generates new projects from the same file. The rule that matters is that it is shared and changes go everywhere at once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
Nothing checked it. A comment stating a policy that no test enforces is a wish, and the six remaining lines all explain a setting that is not obvious from reading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #481 +/- ##
=======================================
Coverage 99.94% 99.94%
=======================================
Files 484 484
Lines 22877 22877
=======================================
Hits 22865 22865
Misses 12 12 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replaces
.golangci.ymlwith a curated set of revive rules, and fixes the reason the old one was not doing what it looked like it was doing.The bug
The file said
version: 2but heldlinters-settingsat the top level, which is the v1 layout.rundrops keys it does not recognise without complaining, soenable-all-rules: truenever applied. Most of revive has been switched off for as long as that line has been there, and nothing said so.golangci-lint config verifyis the check that catches it, and it passes now.What the rules are
Named rules only, not
enable-all-rules. Each one states a convention Go itself has an opinion about:error-naming·error-return·error-strings·errorf·exported·increment-decrement·indent-error-flow·range·receiver-naming·superfluous-else·time-naming·unused-parameter·var-namingRules that measure rather than find are left out.
line-length-limit,cognitive-complexity,cyclomatic,function-lengthandadd-constantall report on a number somebody picked; turning them on here produced 1,363 findings across these repositories, almost all of it naming constants and splitting functions to satisfy a threshold.What it reports today
Zero. That is the intent: this repository already keeps every convention above, so there is nothing to clean up and the config exists to catch the next mistake rather than to schedule work.
Also
Go source vendored inside a JavaScript dependency tree is excluded.
flattedships a Go port beside its JS; it is not tracked in git and only exists afternpm install. It was the only thinggovetreported anywhere.ineffassignjoins the linter list, andmax-issues-per-linter: 0withuniq-by-line: falsestop the output being silently truncated — the default caps at 50 per linter and hides all but the first finding on a line.🤖 Generated with Claude Code
https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c