-
-
Notifications
You must be signed in to change notification settings - Fork 15
ci: add non-required Node 'latest' job; docs: add CHANGELOG entry for Node support updates #277
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
ci: add non-required Node 'latest' job; docs: add CHANGELOG entry for Node support updates #277
Conversation
… Node support updates Co-Authored-By: Kevin Elliott <kevin@welikeinc.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Bug Report
Comments? Email us. |
WalkthroughExtended CI Node.js matrix to include latest in .github/workflows/yarn-test.yml and added a CHANGELOG.md detailing Node.js support policy, CI matrix, tsup target, dev type versions, and two explicit Buffer import fixes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-Authored-By: Kevin Elliott <kevin@welikeinc.com>
Bug Report
Comments? Email us. |
…docs: update changelog wording Co-Authored-By: Kevin Elliott <kevin@welikeinc.com>
|
An error occured. This error may be due to rate limits. If this error persists, please email us. |
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
🧹 Nitpick comments (2)
CHANGELOG.md (2)
10-10: Clarify non-required status and avoid smart quotes.Reflect the CI policy and use straight quotes to avoid typographic inconsistencies.
- - CI matrix updated to test Node 18.x, 20.x, 22.x, 24.x, and “latest” for early signal on upcoming changes + - CI matrix updated to test Node 18.x, 20.x, 22.x, 24.x, and "latest" (non-required) for early signal on upcoming changes
11-11: Tighten grammar to reduce ambiguity and staleness.Prefer “the latest” and consider dropping the pinned “currently v24” to avoid churn.
- - Development types use latest stable `@types/node` (currently v24) as a devDependency + - Development types use the latest stable `@types/node` as a dev dependency
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/yarn-test.yml(1 hunks)CHANGELOG.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...## Unreleased - Node.js support and CI: - Declare runtime support: Node >=18 in `p...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... upcoming changes - Development types use latest stable @types/node (currently ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...rently v24) as a devDependency - Fixes: - Add explicit Buffer imports from `node...
(QB_NEW_EN)
🔇 Additional comments (3)
.github/workflows/yarn-test.yml (1)
20-20: Confirmed — actions/setup-node@v4 acceptsnode-version: latestNo change required: "latest" (aliases: *, current, node) is supported and your matrix entry is valid. Keep explicit majors alongside "latest" for stability.
- Location: .github/workflows/yarn-test.yml — line ~20
- Current: node-version: [18.x, 20.x, 22.x, 24.x, latest]
Optional: use the action's check-latest input to force downloading the absolute newest release (trade-off: performance/caching) or run "latest" in a separate job if you want to isolate potential breakages.
CHANGELOG.md (2)
13-14: LGTM: Clear and actionable fix notes.Explicitly calling out the Buffer imports and file paths is helpful for traceability.
8-9: Changelog verified — package.json and tsup config match the changelogConfirmed:
- package.json — engines.node: ">=18"
- tsup.config.ts — target: "node18" (tsup.config.ts:10)
- CHANGELOG.md — line referencing tsup target
node18(line 9)
| # build against all active LTS and current stable node; include "latest" for early signal as a required job | ||
| node-version: [18.x, 20.x, 22.x, 24.x, latest] |
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.
💡 Verification agent
🧩 Analysis chain
Make “latest” non-required and non-blocking; fix misleading comment.
The PR objective specifies the "latest" job should be non-required. The inline comment says "as a required job," which contradicts that intent. Also, without special handling, failures on "latest" will fail the workflow.
- Update the comment to reflect non-required.
- Make the "latest" matrix run non-blocking with continue-on-error at the job level.
Apply within this hunk:
- # build against all active LTS and current stable node; include "latest" for early signal as a required job
+ # build against all active LTS and current stable Node; include "latest" for early signal as a non-required jobAdd this (outside the hunk, at the job level under runs-on), to avoid blocking PRs on "latest":
# make the "latest" matrix job non-blocking
continue-on-error: ${{ matrix.node-version == 'latest' }}Please also ensure branch protection rules do NOT mark the "Node Test CI / build (latest)" check as required.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo root: $(pwd)"
echo "Looking for workflow file..."
rg -n --hidden --glob '!.git' "^" .github/workflows/yarn-test.yml || true
echo "---- .github/workflows/yarn-test.yml ----"
sed -n '1,240p' .github/workflows/yarn-test.yml || true
echo "---- Search for continue-on-error occurrences ----"
rg -n --hidden "continue-on-error" || trueLength of output: 1995
Make "latest" non-required and non-blocking in CI
The workflow currently includes "latest" in the matrix but neither the comment nor the job prevents it from blocking PRs — update the comment and add a job-level continue-on-error.
Files to change:
- .github/workflows/yarn-test.yml — job: build (matrix around lines ~18–20)
Apply these edits:
Replace the inline comment:
- # build against all active LTS and current stable node; include "latest" for early signal as a required job
+ # build against all active LTS and current stable Node; include "latest" for early signal as a non-required jobAdd a job-level non-blocking rule (place as a sibling to runs-on, at the job level):
build:
runs-on: ubuntu-latest
+ # make the "latest" matrix job non-blocking
+ continue-on-error: ${{ matrix['node-version'] == 'latest' }}(Use bracket notation matrix['node-version'] because the key contains a hyphen.)
Also ensure your branch protection rules do NOT mark the "Node Test CI / build (latest)" check as required.
📝 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.
| # build against all active LTS and current stable node; include "latest" for early signal as a required job | |
| node-version: [18.x, 20.x, 22.x, 24.x, latest] | |
| build: | |
| runs-on: ubuntu-latest | |
| # make the "latest" matrix job non-blocking | |
| continue-on-error: ${{ matrix['node-version'] == 'latest' }} | |
| strategy: | |
| matrix: | |
| # build against all active LTS and current stable Node; include "latest" for early signal as a non-required job | |
| node-version: [18.x, 20.x, 22.x, 24.x, latest] |
🤖 Prompt for AI Agents
.github/workflows/yarn-test.yml around lines 19 to 20: the matrix currently
lists node-version: [18.x, 20.x, 22.x, 24.x, latest] and the inline comment
implies "latest" should be an early signal but it is blocking; update the
comment to state that "latest" is non-blocking/optional, and modify the build
job to add a job-level continue-on-error for the "latest" matrix entry by using
bracket notation matrix['node-version'] when checking for "latest" and setting
continue-on-error: true at the job level (as a sibling to runs-on) only when the
node-version equals "latest"; also update branch protection to ensure the "Node
Test CI / build (latest)" check is not configured as required.
ci: add non-required Node 'latest' job; docs: add CHANGELOG entry for Node support updates
Summary
This is a follow-up to the previous Node.js support matrix update. Adds a Node "latest" job to CI for early signal on upcoming Node versions, plus creates a CHANGELOG.md to document the recent Node support policy changes.
Changes:
latestto the Node version matrix in.github/workflows/yarn-test.ymlalongside existing 18.x/20.x/22.x/24.x jobsCHANGELOG.mdwith entry documenting the Node ≥18 support policy, tsup node18 targeting, Buffer import fixes, and CI matrix updates from the previous PRReview & Testing Checklist for Human
Diagram
%%{ init : { "theme" : "default" }}%% graph TD A[".github/workflows/yarn-test.yml"]:::major-edit B["CHANGELOG.md"]:::major-edit C["CI Matrix Jobs"]:::context D["Node 18.x"]:::context E["Node 20.x"]:::context F["Node 22.x"]:::context G["Node 24.x"]:::context H["Node latest"]:::minor-edit A --> C C --> D C --> E C --> F C --> G C --> H subgraph Legend L1[Major Edit]:::major-edit L2[Minor Edit]:::minor-edit L3[Context/No Edit]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
Session Info:
Summary by CodeRabbit
Bug Fixes
Documentation
Chores