lint: exclude revive var-naming instead of suppressing it per file - #27
kmannislands wants to merge 2 commits into
Conversation
|
| Branch | Total Count |
|---|---|
| main | 217 |
| This PR | 219 |
| Difference | +2 (0.92%) |
📁 Changes by file type:
| File Type | Change |
|---|---|
| Go files (.go) | ❌ +2 |
| Documentation (.md) | ➖ No change |
| Earthfiles | ➖ No change |
Keep up the great work migrating from Earthly to Earthbuild! 🚀
💡 Tips for finding more occurrences
Run locally to see detailed breakdown:
./.github/scripts/count-earthly.shNote that the goal is not to reach 0.
There is anticipated to be at least some occurences of earthly in the source code due to backwards compatibility with config files and language constructs.
Protobuf packages are named by the .proto, so revive's var-naming rule fires on every file that declares one, and fourteen files carried a //nolint:revive to silence it. copy.go in #24 had to grow one too, which is what prompted this. Upstream already solved it in config -- origin/master's exclude list is ours plus a var-naming entry -- and converted the remaining directives to //nolint:staticcheck, which is a newer staticcheck absorbing stylecheck's ST1003. Our pinned golangci-lint 1.55 keeps stylecheck separate and does not enable it, so nothing replaces the directives here. Removing them is not optional once the exclusion lands: nolintlint is enabled, and an unused directive is itself a lint failure. The set is the one upstream converted -- package clauses, _LOOP_CTL_GET_FREE, and IsSolve_Subject -- verified by running revive 1.3.2, the version 1.55 bundles, with var-naming alone. dockerfile_test.go keeps its directive: that one is context-as-argument, a different rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9ca964f to
bbdbcd0
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
💤 Files with no reviewable changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe lint configuration adds an exclusion rule for ChangesRevive lint updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change only adjusts lint configuration and source suppressions. It introduces no runtime risk and is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 14 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.13.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
1 participant
Stacked on #24.
Protobuf packages are named by the
.proto, so revive'svar-namingfires on every file declaring one. Fourteen files carried a//nolint:revivefor it, and #24 had to add a fifteenth — which is what prompted this.What upstream does:
origin/master's revive exclude list is identical to ours plus avar-namingentry. Upstream then converted the leftover directives to//nolint:staticcheck, because a newer staticcheck absorbed stylecheck's ST1003, which flags the same thing. Our pinned golangci-lint 1.55 keepsstylecheckseparate and doesn't enable it, so nothing needs to replace the directives here.Removing them isn't optional once the exclusion lands:
nolintlintis enabled, so an unused directive is itself a failure.The set matches the one upstream converted — package clauses,
_LOOP_CTL_GET_FREE,IsSolve_Subject— verified by running revive 1.3.2 (the version 1.55 bundles) withvar-namingalone.dockerfile_test.gokeeps its directive; that one iscontext-as-argument, a different rule.Net effect on the fork delta: fifteen lines closer to upstream, not further.
🤖 Generated with Claude Code
Summary by CodeRabbit