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
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.
✅ Deploy Preview for devsydev canceled.
|
✅ Deploy Preview for images-devsy-sh canceled.
|
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 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 |
The golangci-lint-ci-parity hook now runs at the pre-commit stage, but the Pre-commit CI job (j178/prek-action) never installed the task CLI it depends on, causing task cli:lint:ci to fail with "No such file or directory".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
golangci-lint-ci-parity's git merge-base HEAD origin/main needs shared commit history, but actions/checkout defaults to a shallow clone, so the hook failed with "unknown revision or path not in the working tree" in the Pre-commit job.
golangci-lint-fmt installs its own private golangci-lint copy for that hook only, but the system-language golangci-lint-ci-parity hook invokes the golangci-lint binary directly and needs it on PATH, which the Pre-commit job never provided.
1 participant
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