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

Migrate dev environment and workflows to node16 #1278

Merged
merged 16 commits into from
Jan 3, 2023

Conversation

fhammerl
Copy link
Contributor

@fhammerl fhammerl commented Dec 20, 2022

To fix critical audit vulnerabilities, we needed to upgrade (dev) dependencies to versions that are no longer compatible with our current workflows using node12.

Node12 and node16 are the versions used by the runner. With Node12 being deprecated, we've targeted node16 for this project as well. It makes sense to keep in sync with the runner, as most eventual applications of actions/tookit's packages end up being executed by the runner.

Also contains numerous fixes to satisfy audit-all

@fhammerl fhammerl requested review from a team as code owners December 20, 2022 14:25
Copy link
Contributor

@johnsudol johnsudol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fhammerl fhammerl changed the title Bump workflows to node16 (the version used by the runner) and apply fixes to prevent regressions Migrate dev environment and workflows to node16 Dec 22, 2022
@@ -219,6 +219,13 @@ export class DownloadHttpClient {
fileDownloadPath: string
): Promise<void> => {
destinationStream.close()
// await until file is created at downloadpath; node15 and up fs.createWriteStream had not created a file yet
Copy link
Contributor Author

@fhammerl fhammerl Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node16 regression:

The single code change we need in node16: awaiting destinationStream.on('close', ensures that the file has been created so we can rmFile delete it on line 229.

@@ -24,10 +24,10 @@ jobs:
- name: Checkout
uses: actions/checkout@v2

- name: Set Node.js 12.x
- name: Set Node.js 16.x
uses: actions/setup-node@v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setup-node can also be updated to v3?

This was referenced Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants