CI: Test Node 20.x, 22.x (Active LTS), 24.x (Current); gate coverage to 22.x#18
CI: Test Node 20.x, 22.x (Active LTS), 24.x (Current); gate coverage to 22.x#18cloudbring merged 3 commits intomasterfrom
Conversation
…gate coverage/artifacts to 22.x
|
Caution Review failedThe pull request is closed. WalkthroughThe CI workflow configuration was updated to test against Node.js versions 20.x, 22.x, and 24.x. Coverage upload and artifact steps were restricted to only execute on Node.js 22.x, instead of previously running on 20.x or unconditionally. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit ignore |
|
Tip For best results, initiate chat on the files or code changes. @cloudbring, could you please clarify what you'd like me to ignore? Here are some options:
Let me know what you'd like to exclude from the review, and I'll help you with the appropriate approach. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/ci.yml (1)
37-43: Limit integration runs to a single Node versionThe integration step will already skip when
NEW_RELIC_API_KEYis unset, so we only need to guard against running it on every matrix entry (and thus duplicating external calls). In.github/workflows/ci.yml(around lines 37–43), update the step to:- name: Run tests with coverage (and integration) + if: | + matrix.node-version == '22.x' && + secrets.NEW_RELIC_API_KEY != '' && + vars.NEW_RELIC_ACCOUNT_ID != '' env: USE_REAL_ENV: true NEW_RELIC_API_KEY: ${{ secrets.NEW_RELIC_API_KEY }} NEW_RELIC_ACCOUNT_ID: ${{ vars.NEW_RELIC_ACCOUNT_ID }} run: npm run ai:test:coverageIf you still want pure unit/coverage runs on other Node versions, add a separate step (without
USE_REAL_ENV) for those matrix entries.
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
14-14: Prevent cascade cancellations across the expanded matrix.Consider disabling fail-fast so a failure on one Node version doesn’t cancel the others.
strategy: + fail-fast: false matrix: node-version: [20.x, 22.x, 24.x]
54-59: Artifact upload scoped to 22.x makes sense.Consider explicitly handling missing files to avoid noisy warnings if earlier steps fail.
- name: Upload HTML coverage as artifact - if: always() && matrix.node-version == '22.x' + if: always() && matrix.node-version == '22.x' uses: actions/upload-artifact@v4 with: name: coverage-html path: coverage + if-no-files-found: warn retention-days: 7
82-85: Publish with Node 22.x to match the tested LTS baseline.You test and upload coverage on 22.x (Active LTS) but publish with 20.x. Aligning the publish environment with 22.x reduces drift risk.
- uses: actions/setup-node@v4 with: - node-version: '20.x' + node-version: '22.x' registry-url: 'https://registry.npmjs.org'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (24.x)
🔇 Additional comments (2)
.github/workflows/ci.yml (2)
14-14: Matrix update aligns with Node release policy — looks good.20.x (Maintenance), 22.x (Active LTS), 24.x (Current) coverage is appropriate.
62-69: Appending coverage summary only on 22.x is appropriate.This keeps the job summary clean and non-duplicative.
…es input; keep uploads on 22.x only
…ation to 22.x, Codecov v4 with OIDC, artifact warn, publish on 22.x)
Rationale: Aligns with Node release policy (22.x Active LTS, 24.x Current, 20.x Maintenance LTS) and keeps CI efficient.
Summary by CodeRabbit