Skip to content
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

fix: Disallow Node.js major version >20 #9630

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Conversation

gibson042
Copy link
Member

refs: #9622

Description

#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

Security Considerations

None.

Scaling Considerations

n/a

Documentation Considerations

Matches the README.

Testing Considerations

Checked by integration testing in CI.

Upgrade Considerations

Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).

@gibson042 gibson042 added the force:integration Force integration tests to run on PR label Jul 1, 2024
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah bad copy paste, I'm sorry!

Copy link
Contributor

@siarhei-agoric siarhei-agoric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@gibson042 gibson042 added the automerge:rebase Automatically rebase updates, then merge label Jul 1, 2024
@mergify mergify bot merged commit caaec05 into master Jul 1, 2024
90 of 93 checks passed
@mergify mergify bot deleted the gibson-9623-followup branch July 1, 2024 18:07
gibson042 pushed a commit that referenced this pull request Jul 1, 2024
refs: #9622

## Description
#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

### Security Considerations
None.

### Scaling Considerations
n/a

### Documentation Considerations
Matches the [README](https://github.com/Agoric/agoric-sdk?tab=readme-ov-file#prerequisites).

### Testing Considerations
Checked by integration testing in CI.

### Upgrade Considerations
Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).
gibson042 added a commit that referenced this pull request Jul 1, 2024
Rebase todo

```
# Branch fix-vow-include-vat-js-in-package-files-9607-
label base-fix-vow-include-vat-js-in-package-files-9607-
pick b6ffa6f fix(vow): include vat.js in package files
label fix-vow-include-vat-js-in-package-files-9607-
reset base-fix-vow-include-vat-js-in-package-files-9607-
merge -C a3826e9 fix-vow-include-vat-js-in-package-files-9607- # fix(vow): include vat.js in package files (#9607)

# Pull Request #9601
pick 6bc363b fix(cosmos): only allow snapshot export at latest height (#9601)

# Branch Force-xsnap-rebuild-9618-
label base-Force-xsnap-rebuild-9618-
pick 467435a fix(xsnap): force rebuild if build config changes
pick a22772e fix(agd): check xsnap was rebuilt
pick 2b53896 feat(xsnap): force rebuild if binary version mismatch
label Force-xsnap-rebuild-9618-
reset base-Force-xsnap-rebuild-9618-
merge -C 78b6fec Force-xsnap-rebuild-9618- # Force xsnap rebuild (#9618)

# Branch agd-enforce-Node-js-version-9623-
label base-agd-enforce-Node-js-version-9623-
#pick 4b35caf revert fix: typescript-estree does not support Node.js LTS (#9619)
pick 5f01bef fix(agd): force own node.js version check
label agd-enforce-Node-js-version-9623-
reset base-agd-enforce-Node-js-version-9623-
merge -C 4f70e66 agd-enforce-Node-js-version-9623- # agd enforce Node.js version (#9623)

# Branch gibson-9623-followup
label base-gibson-9623-followup
pick 6102eb9 fix: Disallow Node.js major version >20
label gibson-9623-followup
reset base-gibson-9623-followup
merge -C caaec05 gibson-9623-followup # (upstream/master) fix: Disallow Node.js major version >20 (#9630)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants