-
Notifications
You must be signed in to change notification settings - Fork 424
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
deps(ci node): update node in ci to 18.x [WIP] #5810
base: master
Are you sure you want to change the base?
Conversation
Tried bumping the vscode/electron-test version due to failing windows tests, but perhaps its unrelated. I'll continue to investigate. |
Co-authored-by: Justin M. Keyes <[email protected]>
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!
We need to pass
{ shell: true }
to avoid error on windows. see more here: https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
related: #5292
Problem
There's no real reason we need to use node 16 in CI, and it's preventing us from using some features.
Our tests run against vscode's own version of node, which is unrelated to the node we use to run scripts, etc. (See also: #5761 )
Solution
update CI runners to node 18 instead of 16.
We need to pass
{ shell: true }
to avoid error on windows. see more here: https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2License: I confirm that my contribution is made under the terms of the Apache 2.0 license.