fix(lint): run CI-parity lint check at pre-commit, not just pre-push - #886
fix(lint): run CI-parity lint check at pre-commit, not just pre-push#886skevetter wants to merge 4 commits into
Conversation
✅ Deploy Preview for devsydev canceled.
|
✅ Deploy Preview for images-devsy-sh canceled.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pre-commit CI job now sets up Go from ChangesPre-commit CI updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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)
.github/workflows/pr-ci.yml (1)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Task CLI version used by CI.
@latestselects a changing upstream release. A new release can change Task behavior or toolchain requirements, so CI can change without a repository commit. Use a tested exactv3.x.yversion and update it deliberately. Go supports both latest and version-specific module installation. (go.dev)🤖 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 @.github/workflows/pr-ci.yml at line 49, Update the Task CLI installation command in the CI workflow to use a tested exact v3.x.y version instead of `@latest`, preserving the existing go install invocation and allowing future version changes only through deliberate commits.
🤖 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 @.pre-commit-config.yaml:
- Line 68: The pre-commit stage currently invokes the lint task that
unconditionally runs git fetch, blocking commits without network access. Update
the hook configuration around the stages entry so Go linting in pre-commit uses
a local or cached base without fetching, while retaining the fetch-dependent
validation in pre-push or CI.
---
Nitpick comments:
In @.github/workflows/pr-ci.yml:
- Line 49: Update the Task CLI installation command in the CI workflow to use a
tested exact v3.x.y version instead of `@latest`, preserving the existing go
install invocation and allowing future version changes only through deliberate
commits.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe93d263-cde1-40f4-be42-1b39cf8e65b3
📒 Files selected for processing (2)
.github/workflows/pr-ci.yml.pre-commit-config.yaml
| types: [go] | ||
| pass_filenames: false | ||
| stages: [pre-push] | ||
| stages: [pre-commit, pre-push] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Avoid an unconditional network fetch in the pre-commit path.
This stage now runs task cli:lint:ci during pre-commit. That Taskfile task executes git fetch --quiet origin main before linting. Developers without network access, valid Git credentials, or an origin/main branch will be unable to create commits that change Go files.
Keep the fetch in the pre-push or CI path, or provide a pre-commit path that uses a cached base with an explicit stale-base policy.
🤖 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 @.pre-commit-config.yaml at line 68, The pre-commit stage currently invokes
the lint task that unconditionally runs git fetch, blocking commits without
network access. Update the hook configuration around the stages entry so Go
linting in pre-commit uses a local or cached base without fetching, while
retaining the fetch-dependent validation in pre-push or CI.
Up to standards ✅🟢 Issues
|
a2efd16 to
3c7cc6e
Compare
Removes the plain golangci-lint hook, which lints whole packages and surfaces pre-existing findings unrelated to the diff. The golangci-lint-ci-parity hook already reproduces CI's exact new-from-patch behavior via `task cli:lint:ci`; running it at pre-commit (in addition to pre-push) catches CI-breaking issues like formatter violations before they're pushed.
3c7cc6e to
8614db1
Compare
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Summary
golangci-lint-ci-parityhook (task cli:lint:ci, which mirrors CI's--new-from-patchdiff-vs-origin/main behavior) atpre-commitin addition topre-push, so CI-breaking issues (e.g. formatter violations) are caught before the first commit rather than only at push time.golangci-linthook, which lints entire packages with no diff scoping and surfaces pre-existing, unrelated findings on any touched file — noisy and not representative of what CI actually enforces.Verified locally: reverting to a pre-fix commit and re-running
pre-commit run golangci-lint-ci-parity --hook-stage pre-commitreproduces the exact golines failure PR #885's CI hit; with the fix applied, the hook passes.Summary by CodeRabbit