-
Notifications
You must be signed in to change notification settings - Fork 9
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
refactor to more generic code #12
Conversation
CC @nodejs/build @nodejs/releasers |
.github/workflows/ci.yml
Outdated
@@ -5,7 +5,7 @@ on: | |||
branches: master | |||
|
|||
env: | |||
NODE_VERSION: 12.x | |||
NODE_VERSION: 18.x |
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.
We should keep testing on Node.js 12 until we can upgrade the version of Node.js used to run this tool on our server.
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.
well, that breaks using modern javascript async/await ect :/
What is needed to upgrade node.js on the www server?
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.
Current server is running Ubuntu 16.04 and using nodesource to install Node.js. I'm not sure whether Nodesource provide later versions of Node.js on Ubuntu 16.04. I'd like to update the OS on the server but it's risky given how dependent we are on that droplet at the moment.
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.
so we would either need to adjust this to run on node 12 or upgrade node without using nodesource, since upgrading the OS is not trivial
I prefer the second option, is that possible?
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 Node.js 18 is a non-starter on Ubuntu 16.04 due to glibc versions. Node.js 16 is a possibility -- there may even be a Nodesource distribution for it (I'd suggest testing in a separate Ubuntu 16.04 system first).
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.
@MoLow Yes, it should be.
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.
Based on the fact that https://deb.nodesource.com/node_16.x/dists/xenial/Release returns something, I think v16.x can be installed using the NodeSource distribution.
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 was able to install node16 on ubuntu 16.04 using nodesource. let me check these changes work on node 16
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.
tests have passed. @richardlau should I go ahead and update www nodejs version?
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.
@MoLow Yes (and keep an eye on tomorrow's nightly to check the indexer still runs ok). If you could also update https://github.com/nodejs/build/blob/b8b63ade1e76d8ccf669335ee384ed2ee34a3abe/ansible/www-standalone/tasks/base.yaml#L1-L3 (which says Node.js 8 but we are definitely running with 12 on the server) that would be great.
I'v updated the www server to node 16. how does this package get published to npm? |
ah I'll need to add you as a maintainer in npm. I've just invited you via npm's web ui -- if it needs to be done via the CLI (we had some issues adding someone to citgm in npm) I'll have to do that tomorrow. |
A recent refactor of `nodejs-latest-linker` has moved the mapping between Node.js versions and codenames to a different file. Refs: nodejs/nodejs-latest-linker#12
A recent refactor of `nodejs-latest-linker` has moved the mapping between Node.js versions and codenames to a different file. Refs: nodejs/nodejs-latest-linker#12 PR-URL: #50299 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
A recent refactor of `nodejs-latest-linker` has moved the mapping between Node.js versions and codenames to a different file. Refs: nodejs/nodejs-latest-linker#12 PR-URL: #50299 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
A recent refactor of `nodejs-latest-linker` has moved the mapping between Node.js versions and codenames to a different file. Refs: nodejs/nodejs-latest-linker#12 PR-URL: nodejs#50299 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
A recent refactor of `nodejs-latest-linker` has moved the mapping between Node.js versions and codenames to a different file. Refs: nodejs/nodejs-latest-linker#12 PR-URL: #50299 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Ref: nodejs/release-cloudflare-worker#33
in order to get Cloudflare to link these soft links, I've refactored the code to be more generic, this way we can use in CF workers: