docs(wiki): add consumer lint-config guidance - #927
Merged
Conversation
Services built on GoBricks have no way to inherit its lint posture: golangci-lint v2 has no `extends` and no `include` (verified — the key fails schema validation with "additional properties 'extends' not allowed"). A sub-module walks up to the nearest .golangci.yml, which is how tools/migration inherits ours, but across repositories the only option is to copy the file. wiki/linting.md covers the copy: which parts are portable, the three edits a consumer must make (gci prefix, delete the framework-only exclusions, trim the importas map), and the settings calibrated for a framework rather than a service (dupl 100, gocyclo 15, lll 215 — the Developer Manifesto already says apps are held to a different standard). The more useful half is the part a template file cannot carry: - The nine linters measured and REJECTED, with reasons. ireturn (663) and wrapcheck (321) are standard picks that fight this framework specifically: database.Interface and messaging.AMQPClient are returned as interfaces by design, and wrapcheck is the direct inverse of wrap-once-at-boundaries. Consumers inherit those signatures, so the reasoning transfers even though the counts do not. - The three measurement traps that each produced a wrong number during our own adoption: issues.uniq-by-line defaults true and undercounts (perfsprint read 44 in a combined run, 149 alone, because err113 and wrapcheck claimed the same fmt.Errorf lines); an unknown revive rule name logs level=error and still exits 0; revive.rules replaces the default set instead of extending it. - One reversed verdict. go-ruleguard was rejected here because a public library should not push a lint-only go.mod dependency into every consumer's module graph. That objection does not apply to a service, so the page says ruleguard is a reasonable choice for them, rather than presenting our decision as universal. Commands in the page were run, not composed: `golangci-lint fmt --diff ./... | grep -c '^--- '` under the naive `standard, default` order returns 197, which independently reproduces the file count already cited in .golangci.yml's own section-order rationale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdded a linting guide that covers configuration, adoption, measurement, formatter behavior, and related documentation. Added the guide to the reference documentation links. ChangesLinting Documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiki/linting.md`:
- Around line 113-120: Update the ruleguard configuration guidance in the
section discussing failOn so it requires import errors to fail as well as DSL
errors, using failOn: dsl,import or all. Preserve the existing warning about
rules-file loading failures and avoid recommending failOn: dsl alone.
- Line 108: Update the `revive: datarace` row in `wiki/linting.md` to replace
the claim that `go test -race` covers the class soundly with wording that states
it detects races only on exercised paths and does not prove their absence.
- Around line 144-147: Update the revive.rules guidance to state that this
replacement behavior applies when enable-default-rules is omitted, and document
that setting enable-default-rules: true preserves the defaults without
re-declaring them.
- Around line 166-168: Update the build-tag explanation in the linting
documentation to state that golangci-lint run omits integration-tagged files
only when invoked without --build-tags=integration; acknowledge that
matching-tag invocations can analyze them while preserving the distinction from
fmt’s file-based behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3835f1d4-0959-4028-9dfc-723983eaa8dd
📒 Files selected for processing (2)
CLAUDE.mdwiki/linting.md
All four CodeRabbit findings were the same defect: behavior true for THIS repo stated as universal, in a page written for other repositories. - `go test -race` "covers the class soundly" overclaimed. The detector only reports races on paths a test actually executes and cannot prove absence. - ruleguard `failOn: dsl` is insufficient. It catches DSL syntax errors but still logs-and-skips on an unresolvable import — the likeliest failure, since the rules file must import go-ruleguard/dsl. Now `dsl,import`. - `revive.rules` replaces the default set only when `enable-default-rules` is omitted. Verified `enable-default-rules: true` is a valid key at v2.12.2 (config verify exits 0); documented it as the shorter path for a new config. - `run` "never sees" build-tagged files was too broad. It sees them when passed a matching --build-tags. GoBricks' make lint and CI jobs do not pass it, which is why fmt is load-bearing here; consumers must check their own invocations. Follow-up worth its own PR: .golangci.yml re-declares all 23 revive defaults because that was the only mechanism known when it was written. `enable-default-rules: true` would replace those 23 lines, and removes the failure mode where dropping one silently loses enforcement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiki/linting.md`:
- Around line 147-157: Update the revive YAML example to show the required
golangci-lint v2 nesting under linters.settings, or explicitly label it as an
indented fragment. Preserve the existing enable-default-rules and rules content
while making the configuration context clear.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7fe16fee-4eff-4b2d-82ad-d83c90a0cbcb
📒 Files selected for processing (1)
wiki/linting.md
The revive and importas snippets started at the linter name with no parent, so pasting either into a .golangci.yml puts it at top level, where v2 rejects it. Both now show the full linters.settings path. The gci example was already correct. Verified rather than eyeballed: all three snippets concatenated into one file pass `golangci-lint config verify` at the pinned v2.12.2 (exit 0). A docs page whose whole purpose is to be copied should not ship YAML that has never been parsed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wiki/linting.md (1)
137-139: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a shell-safe linter placeholder.
If a reader copies this command literally, the shell parses
<linter>and>as redirection operators. Use a concrete example or a shell variable.Proposed fix
-golangci-lint run --enable-only=<linter> --uniq-by-line=false ./... +LINTER=revive +golangci-lint run --enable-only="$LINTER" --uniq-by-line=false ./...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiki/linting.md` around lines 137 - 139, Update the golangci-lint command example in the linting documentation to replace the angle-bracket <linter> placeholder with a shell-safe concrete linter name or variable, while preserving the existing --enable-only, --uniq-by-line, and ./... arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@wiki/linting.md`:
- Around line 137-139: Update the golangci-lint command example in the linting
documentation to replace the angle-bracket <linter> placeholder with a
shell-safe concrete linter name or variable, while preserving the existing
--enable-only, --uniq-by-line, and ./... arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a7a41ad-2015-4eef-92a5-042e6c49ed36
📒 Files selected for processing (1)
wiki/linting.md
`--enable-only=<linter>` is not a placeholder in a bash block, it is shell redirection — pasting the line as written fails before golangci-lint runs. Replaced with a LINTER variable, which is both shell-safe and names the one-linter-at-a-time intent the surrounding paragraph is arguing for. The --enable-only, --uniq-by-line and ./... arguments are unchanged. Ran the block verbatim to confirm it executes rather than merely parses. Checked the page's other bash blocks for the same defect: the only remaining angle brackets are `2>&1`, which is real redirection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gaborage
enabled auto-merge (squash)
August 8, 2026 03:18
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
Services built on GoBricks cannot inherit its lint posture — golangci-lint v2 has no
extendsand noinclude, so the only option is copying.golangci.yml.wiki/linting.mdcovers that copy: which parts are portable, the three required edits (gci prefix, delete the
framework-only exclusions, trim the importas map), and which thresholds are calibrated for a
framework rather than a service.
Impact
None — documentation only, plus one line in CLAUDE.md's wiki index. Consumers get the nine
measured-and-rejected linters with reasons (
ireturnandwrapcheckfight this framework'sinterface-returning API specifically), and one reversed verdict:
go-ruleguardwas rejectedhere to keep a lint-only dependency out of consumers' module graphs, which is not a reason
that applies to their repos.
Verification
Commands in the page were run, not composed: the naive
gciorder reproduces 197 rewrittenfiles, independently matching the count already cited in
.golangci.yml's section-orderrationale.
Summary by CodeRabbit