update for node24#145
Conversation
jest works if you set env variables, but can't test because on linux
📝 WalkthroughWalkthroughNode.js runtime bumped from 20 → 24 across configs and metadata; dependencies upgraded; a Jest test file was removed; action.yml typo fixed; getVersion() fallback in src/main.js now substitutes 0 when GITHUB_RUN_NUMBER is unset. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 Tip CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.OpenGrep is compatible with Semgrep configurations. Add an |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
80-80: Note: Jest 30 and Linux compatibility.The PR description mentions Jest fails on Linux, blocking local test execution. This may be related to the Jest 30 upgrade or CommonJS/ESM module resolution in Node 24. The contributor notes environment variables need to be set for Jest to work.
Consider documenting the required environment configuration or investigating the root cause before merging if CI testing is critical.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 80, The Jest upgrade ("jest": "^30.3.0") appears to cause Linux/local test failures (likely Node24 CJS/ESM or env var expectations); either revert/pin the "jest" dependency to the last-known-good version, or reproduce the failure locally and fix module/resolution/config (jest.config or transform settings) and update CI; additionally add a short note in README/CONTRIBUTING about required environment variables and Node/Jest compatibility so contributors can run tests locally. Locate the "jest" entry in package.json and update dependency or add documentation and CI fixes accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.node-version:
- Line 1: Replace the partial Node version string "24.11" in .node-version with
the full semver form "24.11.0" to explicitly pin the Node runtime; update the
file contents so any version manager or CI tooling reads the complete semantic
version.
---
Nitpick comments:
In `@package.json`:
- Line 80: The Jest upgrade ("jest": "^30.3.0") appears to cause Linux/local
test failures (likely Node24 CJS/ESM or env var expectations); either revert/pin
the "jest" dependency to the last-known-good version, or reproduce the failure
locally and fix module/resolution/config (jest.config or transform settings) and
update CI; additionally add a short note in README/CONTRIBUTING about required
environment variables and Node/Jest compatibility so contributors can run tests
locally. Locate the "jest" entry in package.json and update dependency or add
documentation and CI fixes accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cfbf9111-0629-4277-bb1d-24746359e9e3
⛔ Files ignored due to path filters (4)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.node-version__tests__/wait.test.jsaction.ymlpackage.jsonsrc/main.js
💤 Files with no reviewable changes (1)
- tests/wait.test.js
bro how much water did you drain to make a comment that said that I _need_ to put a ".0" here despite 24.11 being enough (dawg it's a minor version, on an LTS, it's not going to magically explode). I feel with like enough context you'd know this, but you're probably too dumb af. I'll do it anyways so you'll stop burning down the forests.
|
Thanks, I'll look at this later next week! |
plugin works in my workflows but can't run the full pipeline on my linux runner locally because jest gets angry (gh core/actions requires you to have all env variables setup before run, but windows you can inject), but works totally fine on windows, yahoo.
This should probably be moved to ESM at some point in the future, as the versions I've pinned to, are the last to support CJS.
Summary by CodeRabbit
Chores
Bug Fixes
Tests