chore: separate go lint scripts#3933
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new fast GolangCI-Lint configuration and updates the Makefile to support a configurable lint path plus two new lint targets for fast linting and style checks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
9-9: Keeplint-gopinned to the full repo.Now that Line 191 honors
GO_LINT_PATH,make lintandmake cican silently skip packages whenever that variable is set in the caller’s environment. I’d keeplint-gohardcoded to./...and reserveGO_LINT_PATHfor the new local-onlylint-go-fast/lint-go-styletargets.Suggested tweak
.PHONY: lint-go lint-go: ## Lint Go code $(call print-target) - golangci-lint run -v $(GO_LINT_PATH) + golangci-lint run -v ./...Also applies to: 188-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 9, The lint target currently uses GO_LINT_PATH which allows callers to skip packages; change the lint-go target to hardcode the full repo path ./... (keep GO_LINT_PATH defined for other targets) so lint-go always runs across the repo, and update lint-go-fast and lint-go-style to reference GO_LINT_PATH for local-only scans; adjust target definitions (lint-go, lint-go-fast, lint-go-style) accordingly so GO_LINT_PATH is reserved for fast/style targets while lint-go uses ./....
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.golangci-fast.yaml:
- Around line 3-44: The fast profile currently enables the same linters as the
full profile; update the linters.enable list (the "linters.enable" block) to a
smaller subset (e.g., keep only fast-running checks like govet, errcheck,
ineffassign, whitespace, misspell) and reduce the staticcheck scope in the
"staticcheck.checks" list (remove "all" and explicitly include only selected
checks or prefix with "-" to disable heavy checks) so lint-go-fast actually runs
fewer checks; also consider trimming exclusions.paths or presets only if needed
to avoid duplicating the main config while ensuring the fast profile is a
lighter, faster subset of the full profile.
---
Nitpick comments:
In `@Makefile`:
- Line 9: The lint target currently uses GO_LINT_PATH which allows callers to
skip packages; change the lint-go target to hardcode the full repo path ./...
(keep GO_LINT_PATH defined for other targets) so lint-go always runs across the
repo, and update lint-go-fast and lint-go-style to reference GO_LINT_PATH for
local-only scans; adjust target definitions (lint-go, lint-go-fast,
lint-go-style) accordingly so GO_LINT_PATH is reserved for fast/style targets
while lint-go uses ./....
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b9afd901-aab7-4069-b67b-74839818a2fe
📒 Files selected for processing (2)
.golangci-fast.yamlMakefile
| linters: | ||
| default: none | ||
| enable: | ||
| - bodyclose | ||
| - errcheck | ||
| - govet | ||
| - ineffassign | ||
| - misspell | ||
| - nolintlint | ||
| - sloglint | ||
| - staticcheck | ||
| - unconvert | ||
| - whitespace | ||
| settings: | ||
| misspell: | ||
| locale: US | ||
| nolintlint: | ||
| require-specific: true | ||
| revive: | ||
| confidence: 0 | ||
| staticcheck: | ||
| checks: | ||
| - "all" | ||
| - "-QF1008" | ||
| - "-SA1019" # https://staticcheck.dev/docs/checks/#SA1019 | ||
| # FIXME: re-enable the following checks | ||
| - "-ST1003" | ||
| - "-ST1016" | ||
| exclusions: | ||
| generated: lax | ||
| presets: | ||
| - comments | ||
| - common-false-positives | ||
| - legacy | ||
| - std-error-handling | ||
| paths: | ||
| - .dagger/internal | ||
| - .dagger/dagger.gen.go | ||
| - collector/benthos/internal | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ |
There was a problem hiding this comment.
This “fast” profile currently matches the full lint profile.
Lines 3-44 are the same as .golangci.yaml:1-45, so lint-go-fast doesn’t actually slim down the lint surface here; it mostly introduces a second config to keep in sync. If the goal is a lighter local pass, this file needs a smaller enabled linter set and/or a reduced staticcheck scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.golangci-fast.yaml around lines 3 - 44, The fast profile currently enables
the same linters as the full profile; update the linters.enable list (the
"linters.enable" block) to a smaller subset (e.g., keep only fast-running checks
like govet, errcheck, ineffassign, whitespace, misspell) and reduce the
staticcheck scope in the "staticcheck.checks" list (remove "all" and explicitly
include only selected checks or prefix with "-" to disable heavy checks) so
lint-go-fast actually runs fewer checks; also consider trimming exclusions.paths
or presets only if needed to avoid duplicating the main config while ensuring
the fast profile is a lighter, faster subset of the full profile.
afe13b1 to
b50a912
Compare
Overview
make lint-go(no cache)INFO Execution took 3m53.407050875s
make lint-go(fresh cache)INFO Execution took 5.087436208s
make lint-go-fast(no cache)INFO Execution took 52.045313s
make lint-go-fast(fresh cache)INFO Execution took 5.377096875s
Summary by CodeRabbit