-
Benachrichtigungen
You must be signed in to change notification settings - Fork 15.4k
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
add support Node.js@22 in the CI #5627
Conversation
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.
Thanks for doing this @mertcanaltin! Maybe there is an error with Node@22... did the tests pass in local?
I got this error when I was node 22 in my local |
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 failure in 22 is expected, it fails in 21 as well, but because the existing CI locks to specific versions, it passes in that specific versino of 21.
See nodejs/node#51562 upstream, and #5615 internally
In the CI I am working on in #5599, Im not locking to specific Node versions and instead using latest for a given major, which I prefer and makes these kinds of failures more obvious.
Opened a PR to skip this test in the meantime: #5628
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 don't have the ability to approve but LGTM as well
This should pass 22 ci after #5628 merged Might need a rebase or something? |
Adds Node.js@22 support to CI