-
Notifications
You must be signed in to change notification settings - Fork 208
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
chore: Update README prerequisites #9295
Conversation
* we generally support the latest LTS release: use [nvm](https://github.com/nvm-sh/nvm) to keep your local system up-to-date | ||
* Yarn (`npm install -g yarn`) | ||
* gcc-10 or newer, or a compiler with `__has_builtin()` | ||
* gcc >=10, clang >=10, or another compiler with `__has_builtin()` |
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.
clang 10 is very old, but consistent with our releases.
Deploying agoric-sdk with Cloudflare Pages
|
8019441
to
380c068
Compare
@@ -17,10 +17,11 @@ to use. | |||
## Prerequisites | |||
|
|||
* Git | |||
* Node.js LTS (version 16.13.0 or higher) | |||
* Go ^1.20.2 | |||
* Node.js ^18.12 or ^20.9 |
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.
the policy is Node LTS, whatever that may be. let's leave specific version numbers out of the README.
though consider linking from here to https://github.com/Agoric/agoric-sdk/blob/master/docs/node-version.md
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.
Hmm, does that make sense in a list that is specific with respect to other software? I think instead it makes sense to have the README be the source of truth and docs/node-version.md to include instructions for updating it in accordance with Node.js LTS policy. We shouldn't expect someone to be able to use a Node.js version that we haven't actively vetted, even if it is LTS.
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.
I agree with @gibson042, there is a difference between policy, and what is actually tested. The goal of this section is also to be referenced from release notes by permalink, so we should have an explicit statement of what versions are explicitly supported and tested.
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.
I think instead it makes sense to have the README be the source of truth and docs/node-version.md to include instructions for updating it in accordance with Node.js LTS policy
I'm not concerned with which doc is the source of truth, but that any doc is rather than CI or package.json engines which is the actual truth about what is supported. I'm weary of multiple sources of "truth" and the likelihood they get out so sync.
Take that input fwiw. No objections to the PR as is.
Fixes #9294
Description
Update the master README for Node.js v18/v20 and Go 1.20+.
Security Considerations
None.
Scaling Considerations
n/a
Documentation Considerations
This presumes that README.md is the source of truth as suggested in #9294.
Testing Considerations
n/a
Upgrade Considerations
Release commits are expected to have a static snapshot as relevant to them (e.g., release-mainnet1B does not support Node.js v20 AFAIK).