-
Notifications
You must be signed in to change notification settings - Fork 474
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
Bump node versions in CI #716
Conversation
BTW, I see we still have a branch called |
When we merge this, we should make I already made slight changes to required jobs:
|
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.
Nice, just one suggestion. Maybe we could have a docker image parameter, like we have in the solidity repo, but with the default nodejs version set to cimg/node:current
so we would only set specific versions where needed and reduce code repetition, since most of the places uses node:current
already.
parameters:
default-nodejs-docker-image:
type: string
default: "cimg/node:current"
...
hardhat-sample-project:
docker:
- image: << pipeline.parameters.default-nodejs-docker-image >>
Or even something like:
nodejs-defaults: &nodejs-defaults
docker:
- image: cimg/node:current
...
hardhat-sample-project:
<<: *nodejs-defaults
steps:
...
Not sure it's much of an improvement. We use specific versions only once now and the rest is In the more repo it makes more sense because versions are long due to the embedded hash. |
Fair enough. Looks good to me then. And I suspect the hardhat test will fail because of similar problem here: ethereum/solidity#14704. But we can fix that in #717. |
This bumps all minor versions to latest, and also adds a new run on
node:current
(which is node 21).It also bumps node version in Hardhat jobs that were not on
current
only because at the time of node 17 Hardhat did not work with it.Truffle does not work with node 21, but is fine with node 20, which is the current
lts
.