Repository navigation
Conversation
chore: update Node.js versions
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76f68fd120
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const MIN_VERSION = [8, 0, 0]; | ||
| const MAX_VERSION = [24, 99, 99]; | ||
| const MAX_VERSION = [26, 99, 99]; |
There was a problem hiding this comment.
In the checked e2e/nodejs_host BazelCI suite, the fixture named node26_auto_fetch is pinned to 26.3.0 (e2e/nodejs_host/MODULE.bazel:39-40), which this widened cap now adds to NODE_VERSIONS (nodejs/private/node_versions.bzl:4435). _toolchain_extension therefore finds a nonempty built-in entry at nodejs/extensions.bzl:64 and never reaches the fetch_node_repositories branch at lines 66-71, so this suite can pass while automatic SHASUM/facts fetching is broken; update the fixture to a released version absent from the generated map when widening the cap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I wasn't aware of #3919. If this is the preferred way of specifying node version going forward I'm happy to close this PR.
If this PR is wanted i guess I have to update the e2e test?
There was a problem hiding this comment.
I think we still want to maintain this node_versions.bzl file for the time being and your change looks correct, although sorry you may need to resolve conflicts.
IMO (and my tentative plan for all many rulesets) we should still keep a set of hashes to avoid polluting the MODULE .lock file, however maybe we'll only have the "latest patch" of each "LTS semver" and if you want an older or odd version then your .lock file gets some extra data in it.
So maybe this script will change to something along those lines in the next major release, but for now what you've done LGTM
update-node-js-version.jswith newMAX_VERSION26.update-node-js-versions.