-
Notifications
You must be signed in to change notification settings - Fork 0
Split CI into separate workflows per Node version #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Rename ci.yml to format.yml (handles auto-formatting only) - Add ci-node18.yml, ci-node20.yml, ci-node22.yml for testing - Test workflows trigger via workflow_run after Format completes - This prevents duplicate CI runs when auto-format commits - Add Format badge and update CI badges in README
WalkthroughReorganises CI into modular workflows: a standalone Format workflow and three Node-specific CI workflows (Node 18, 20, 22). The top-level Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci-node22.yml:
- Around line 41-46: The Codecov v4 step using codecov/codecov-action@v4 needs
an authentication token to upload coverage; update the action inputs for the
step that currently sets directory/name/fail_ci_if_error to include the token
input pointing to the repository secret (use secrets.CODECOV_TOKEN) so uploads
succeed (i.e., add token: ${{ secrets.CODECOV_TOKEN }} to the
codecov/codecov-action@v4 step).
🧹 Nitpick comments (5)
.github/workflows/ci-node20.yml (2)
15-18: Consider usinghead_shainstead ofhead_branchfor deterministic checkout.Using
head_branchmeans this workflow may check out a different commit than what the Format workflow processed, if new commits are pushed between workflows. This could lead to inconsistent CI results.Suggested fix
- name: Checkout code uses: actions/checkout@v4 with: - ref: ${{ github.event.workflow_run.head_branch }} + ref: ${{ github.event.workflow_run.head_sha }}
29-30: Minor: Step name doesn't match action.The step is named "TypeScript type checking" but runs
npm run lint, which typically includes more than just type checking. Consider renaming to "Lint" or "Run lint checks" for clarity..github/workflows/ci-node18.yml (2)
15-18: Samehead_branchvshead_shaconcern applies here.As noted in the Node 20 workflow, using
head_branchinstead ofhead_shacould lead to checking out different code than what the Format workflow validated.Suggested fix
- name: Checkout code uses: actions/checkout@v4 with: - ref: ${{ github.event.workflow_run.head_branch }} + ref: ${{ github.event.workflow_run.head_sha }}
1-39: Optional: Consider consolidating with reusable workflows.The Node 18, 20, and 22 workflows share nearly identical structure. GitHub Actions supports reusable workflows which could reduce duplication and simplify maintenance.
This is fine for now, but worth considering if the CI logic grows more complex.
.github/workflows/ci-node22.yml (1)
15-18: Samehead_branchvshead_shaconcern applies here.For consistency and determinism, consider using
head_shainstead ofhead_branch.Suggested fix
- name: Checkout code uses: actions/checkout@v4 with: - ref: ${{ github.event.workflow_run.head_branch }} + ref: ${{ github.event.workflow_run.head_sha }}
CI improvements: - Add ci.yml orchestrator that triggers on push/PR - Convert format.yml and ci-node*.yml to reusable workflows (workflow_call) - Format runs first, test workflows depend on it via `needs` - Add Codecov token for coverage uploads - Update README to single CI badge Linting: - Add oxlint for actual linting (separate from type checking) - Rename `lint` script to `type-check` (tsc --noEmit) - Add `lint` and `lint:fix` scripts for oxlint - Fix lint warning: remove unnecessary fallback in spread
Summary by CodeRabbit
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.