ci: gate build and test jobs on pre-commit and lint checks#84
Conversation
Add pre-commit and lint as jobs inside pr-ci.yml and make build-cli and build-ui depend on them via `needs`. This prevents wasting CI runner time on builds and tests when basic quality checks fail.
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughThese changes refactor the CI/CD pipeline by removing a disk-space cleanup step from the lint workflow, introducing two new gate jobs ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
golangci-lint doesn't need extra disk space and this step adds significant time to the job.
Swap the manual pre-commit setup (biome, python, go, golangci-lint, goimports, cache) with j178/prek-action@v2 which handles all of this automatically and more efficiently.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/pr-ci.yml (1)
52-52: Consider gatingintegration-testson the new checks too.
build-uiandbuild-clinow require[precommit, lint-gate], butintegration-testsonly depends onbuild-cli. Sincebuild-clitransitively requires the gates, this is fine today — just be aware that ifbuild-cli'sneedsis ever reorganized, the gate coverage for integration tests could silently drop. A one-line comment or explicitneedsentry would make the intent durable.Also applies to: 84-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-ci.yml at line 52, The integration-tests job currently only lists build-cli in its needs, which relies on precommit and lint-gate indirectly; update the integration-tests job (symbol: integration-tests) to explicitly include needs: [precommit, lint-gate, build-cli] or add a one-line comment above the job documenting the intent to require those gates, so the dependency on precommit and lint-gate (symbols: precommit, lint-gate, build-cli) is durable even if build-cli's needs are reorganized..github/workflows/pre-commit.yml (1)
18-18: LGTM — consolidation looks good.Delegating tool setup (Biome/Python/Go/golangci-lint/goimports/cache) to
j178/prek-action@v2meaningfully simplifies this workflow. One small consideration:prekis a relatively new Rust reimplementation ofpre-commit. If you want stronger supply-chain guarantees, consider pinning the action to a commit SHA rather than the@v2moving tag (same applies to the newprecommitjob inpr-ci.yml).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pre-commit.yml at line 18, The workflow uses the moving tag j178/prek-action@v2 which can change over time; update the invocation to pin the action to a specific commit SHA instead of `@v2` (and do the same for the precommit job in pr-ci.yml) so the exact action revision is locked; locate the reference to j178/prek-action@v2 in the workflow and replace the tag with the stable commit SHA for that release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-ci.yml:
- Around line 10-33: The CI currently runs golangci-lint twice (once via the
precommit job using j178/prek-action@v2 which invokes the golangci-lint
pre-commit hooks, and again in the lint-gate job using
golangci/golangci-lint-action@v9), causing duplicated work and potential version
mismatches; resolve by choosing one source of truth: either remove the
golangci-lint and golangci-lint-fmt hooks from .pre-commit-config.yaml and keep
the lint-gate job, or remove the lint-gate job entirely and make
build-ui/build-cli depend on precommit (needs: [precommit]); if you must keep
both, ensure the golangci-lint version is pinned and consistent between the
precommit hooks and the lint-gate action (align versions in the pre-commit
config and the lint-gate job).
- Line 33: The workflow sets only-new-issues: true globally which also affects
push events; change it so the flag only applies for PR runs: either move the
lint-gate job (or the only-new-issues key) to run only for pull_request events
(make the lint-gate job's runs-on/if conditional target pull_request) or add an
if-condition on the job that checks github.event_name == 'pull_request' and only
sets/enables only-new-issues in that branch; refer to the only-new-issues
setting and the lint-gate job name in your changes.
- Line 32: Replace the non-deterministic "version: latest" setting in the CI
workflow with the pinned golangci-lint release used by pre-commit; update the
workflow entry that currently contains the literal "version: latest" to use
"v2.11.4" so the CI linter matches the version in .pre-commit-config.yaml and
yields reproducible linting results.
---
Nitpick comments:
In @.github/workflows/pr-ci.yml:
- Line 52: The integration-tests job currently only lists build-cli in its
needs, which relies on precommit and lint-gate indirectly; update the
integration-tests job (symbol: integration-tests) to explicitly include needs:
[precommit, lint-gate, build-cli] or add a one-line comment above the job
documenting the intent to require those gates, so the dependency on precommit
and lint-gate (symbols: precommit, lint-gate, build-cli) is durable even if
build-cli's needs are reorganized.
In @.github/workflows/pre-commit.yml:
- Line 18: The workflow uses the moving tag j178/prek-action@v2 which can change
over time; update the invocation to pin the action to a specific commit SHA
instead of `@v2` (and do the same for the precommit job in pr-ci.yml) so the exact
action revision is locked; locate the reference to j178/prek-action@v2 in the
workflow and replace the tag with the stable commit SHA for that release.
🪄 Autofix (Beta)
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
Run ID: eab6b7ca-47a9-47a5-911b-d4f835faa29f
📒 Files selected for processing (3)
.github/workflows/lint.yml.github/workflows/pr-ci.yml.github/workflows/pre-commit.yml
💤 Files with no reviewable changes (1)
- .github/workflows/lint.yml
| precommit: | ||
| name: Pre-commit Gate | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - uses: j178/prek-action@v2 | ||
|
|
||
| lint-gate: | ||
| name: Lint Gate | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: setup Go | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: golangci-lint | ||
| uses: golangci/golangci-lint-action@v9 | ||
| with: | ||
| version: latest | ||
| only-new-issues: true |
There was a problem hiding this comment.
Redundant golangci-lint execution between precommit and lint-gate.
Per .pre-commit-config.yaml (lines 56–60 in the provided context), the golangci-lint and golangci-lint-fmt hooks are configured as pre-commit hooks, which means j178/prek-action@v2 in the precommit job will already run golangci-lint. The separate lint-gate job then runs it again via golangci/golangci-lint-action@v9. This doubles CI time for Go linting and, worse, the two paths can disagree: the pre-commit hook pins v2.11.4 while the action uses version: latest, so a new upstream release can cause one gate to pass and the other to fail on the same SHA.
Recommendation: pick one source of truth. Either
- drop the
golangci-lint/golangci-lint-fmthooks from.pre-commit-config.yamland keeplint-gate, or - drop
lint-gateand rely solely on the pre-commit hook (thenbuild-ui/build-clionly needneeds: [precommit]).
If you keep both, at minimum align the golangci-lint version between them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-ci.yml around lines 10 - 33, The CI currently runs
golangci-lint twice (once via the precommit job using j178/prek-action@v2 which
invokes the golangci-lint pre-commit hooks, and again in the lint-gate job using
golangci/golangci-lint-action@v9), causing duplicated work and potential version
mismatches; resolve by choosing one source of truth: either remove the
golangci-lint and golangci-lint-fmt hooks from .pre-commit-config.yaml and keep
the lint-gate job, or remove the lint-gate job entirely and make
build-ui/build-cli depend on precommit (needs: [precommit]); if you must keep
both, ensure the golangci-lint version is pinned and consistent between the
precommit hooks and the lint-gate action (align versions in the pre-commit
config and the lint-gate job).
| - name: golangci-lint | ||
| uses: golangci/golangci-lint-action@v9 | ||
| with: | ||
| version: latest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if .pre-commit-config.yaml exists and find golangci-lint version
fd -name ".pre-commit-config.yaml" -exec cat {} \;Repository: devsy-org/devsy
Length of output: 289
🏁 Script executed:
# Read the pr-ci.yml file around line 32
cat -n .github/workflows/pr-ci.yml | sed -n '25,40p'Repository: devsy-org/devsy
Length of output: 560
🏁 Script executed:
cat .pre-commit-config.yamlRepository: devsy-org/devsy
Length of output: 1837
Pin golangci-lint to a specific version to ensure reproducible CI.
Using version: latest makes CI non-deterministic: an upstream release can fail previously-green PRs without any code changes. Pin to v2.11.4 to match the version already declared in .pre-commit-config.yaml, ensuring consistent linting across both pre-commit and CI gates.
Proposed change
- name: golangci-lint
uses: golangci/golangci-lint-action@v9
with:
- version: latest
+ version: v2.11.4
only-new-issues: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| version: latest | |
| - name: golangci-lint | |
| uses: golangci/golangci-lint-action@v9 | |
| with: | |
| version: v2.11.4 | |
| only-new-issues: true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-ci.yml at line 32, Replace the non-deterministic
"version: latest" setting in the CI workflow with the pinned golangci-lint
release used by pre-commit; update the workflow entry that currently contains
the literal "version: latest" to use "v2.11.4" so the CI linter matches the
version in .pre-commit-config.yaml and yields reproducible linting results.
| uses: golangci/golangci-lint-action@v9 | ||
| with: | ||
| version: latest | ||
| only-new-issues: true |
There was a problem hiding this comment.
only-new-issues: true also applies on push to main.
This workflow triggers on both pull_request and push: branches: [main] (line 4–5). only-new-issues is designed for PR diffs; on push events it can silently suppress findings you would expect the gate to catch. If the intent is to use this only as a PR gate, consider gating the flag on event type, or scope lint-gate to pull_request only (the standalone lint.yml workflow can continue covering push-to-main).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-ci.yml at line 33, The workflow sets only-new-issues:
true globally which also affects push events; change it so the flag only applies
for PR runs: either move the lint-gate job (or the only-new-issues key) to run
only for pull_request events (make the lint-gate job's runs-on/if conditional
target pull_request) or add an if-condition on the job that checks
github.event_name == 'pull_request' and only sets/enables only-new-issues in
that branch; refer to the only-new-issues setting and the lint-gate job name in
your changes.
Summary
pr-ci.ymland makebuild-cliandbuild-uidepend on them vianeedspre-commit.ymlandlint.ymlworkflows are kept forpushto mainSummary by CodeRabbit