-
Notifications
You must be signed in to change notification settings - Fork 32
[CI][workflows] Use node 20 by default, align GitHub workflows #310
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
Conversation
|
IP Ticket opened (unrelated to this PR I think): [main] INFO A review is required for npm/npmjs/-/ts-jest/29.2.5. |
8950d0c to
29d9d35
Compare
29d9d35 to
8164426
Compare
8164426 to
f1804e3
Compare
.github/workflows/ci-cd.yml
Outdated
| - published | ||
| - published | ||
| schedule: | ||
| - cron: '0 4 * * *' # Runs every day at 4am: https://docs.github.com/en/actions/reference/events-that-trigger-workflows#scheduled-events-schedule |
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.
Why was the cron job added?
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.
It's a test for a theory. In this repo we have the license check on a cron job but recently the nightly license check started to fail, and the repo's main page was still reflecting a successful state (i.e. the state of the last complete CI run, when the last PR was merged). I'm hoping what with both workflows triggered nightly, the status of an eventually failed license check on master will show on the main project page.
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 removed the new cron entry
Prepare for node.js 18 EoL and transition to node 20 as default - start using node 20 for jobs like linting, publishing and license check - Add Node 20 and node 22 to the matrix for "build and test" job - (for now) Keep node 18 as part of the "matrix" for "build and test job". We can remove it once officially EoL The plan is to remove node 18 soon, when it's officially EoL. Signed-off-by: Marc Dumais <[email protected]>
f1804e3 to
a0b1183
Compare
bhufmann
left a comment
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.
Looks good to me. Thanks!
|
Thanks for the review! |
Add node 20,22 for build and tests (keep 18 for now). Use 20 for publish and license check.
What it does
Prepare for node.js 18 EoL and our transition to using node 20 as default.
How to test
Confirm CI still passes.
Follow-ups
Review checklist