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

Draft - Node 20.9.0 upgrade #7626

Closed
wants to merge 3 commits into from
Closed

Conversation

alphaprinz
Copy link
Contributor

@alphaprinz alphaprinz commented Nov 29, 2023

Explain the changes

  1. Upgrade to nodejs 20.9.0
  2. google_cloud.storage.nodejs-common.Util.getUserAgentFromPackageJson no longer exported. Currently commented out.
  3. net-ping/nan - too old. Has drop-in replacement draft in net_utils.js.
  4. nan 2.18.0 - has compiliation warning, had to remove -Werror in gyp. 'IdleNotificationDeadline' is deprecated: Use MemoryPressureNotification() nodejs/nan#953.
  5. encryption tests - namespace creation uses same bucket, for some reason doesn't fail on node18.

Some more deatils on 2-4: https://docs.google.com/document/d/1uDgFDP2_2TYWEwBn5JtjLcStjOmeqYzBwTctY0pRfsc/edit?usp=drive_link
Some more details on 5: https://noobaa.slack.com/archives/C0NSK9ZNE/p1700419078270099

Issues: Fixed #xxx / Gap #xxx

  1. Newer nodejs version.

Testing Instructions:

  1. make tests
  • Doc added/updated
  • Tests added

Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>
Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>
Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>
@liranmauda
Copy link
Contributor

Hi @alphaprinz
Thanks for this PR 🙂

Please divide it into several PRs.

  • The modules upgrade should be in a separate PR, prior to this one.
  • The modules that require node 20 (are there any?) should be with this PR.
  • The replacement of net-ping/nan should also be a separate PR, prior to this one.

Then, and only then we should have the bump to node 20.
This is to prevent backporting issues to previous branches, and downstream issues.

One more question:

  • Does this node version bump the major version of NPM from 9 to 10?
  • If it does, we need to make sure that the downstream builds can handle it, by asking @b-ranto to create a private build from this branch.

@b-ranto
Copy link
Contributor

b-ranto commented Dec 1, 2023

The downstream build passed.

@alphaprinz
Copy link
Contributor Author

Will be split into separate PRs.

@alphaprinz alphaprinz closed this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants