-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Testing: Some GHA workflow improvements when setting up NodeJS #29078
Conversation
This ensures the version of Node.JS is set up prior to configuring or restoring the cache of NPM modules.
Size Change: 0 B Total Size: 1.42 MB ℹ️ View Unchanged
|
It makes a lot of sense to use explicit version numbers for Node.js 👍 I'm wondering if npm won't cause issues at the time of branching for every major WP version. At the moment we might enforce the latest npm version in some actions. We mostly need yo verify. |
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 requires a rebase now that we formatted all yml
files ...
Changes themselves look good.
This ensures the version of Node.JS is set up prior to configuring or restoring the cache of NPM modules.
…into fix/gha-npm-caching
* Some improvements to NPM caching within GHA workflows. This ensures the version of Node.JS is set up prior to configuring or restoring the cache of NPM modules. * This updates the version of `actions/setup-node` used to the latest, 2.x. * Redo formatting updates. (cherry picked from commit e12881c)
Description
While working on some 5.6.2 backports, I ran into a few issues with my tests running on PRs and noticed a few inconsistencies in the GHA workflow files.
First, not all workflows explicitly define the desired version of NodeJS. This results in the workflow run using the version of NodeJS installed by default on the GHA runner container, which can change over time. In the
wp/5.6
branch, NodeJS 12.x was still used, but the runner container had switched to using NodeJS 14.x and this caused failures.Second, when using the
actions/cache
action to cache NPM modules, NodeJS should always be set up prior to using this action. Changing the NodeJS version after configuring caching could result in restoring a cache for a different version of NodeJS, or pushing an incorrect cache that may cause issues elsewhere. This could also result in much faster workflow runs when the cache was being configured first (npm i
would have had to re-run if the cache had incorrect dependency versions for the desired NodeJS version).I also noticed that the
actions/setup-node
action now has av2
release available. I also updated all steps using this action to now usev2
.How has this been tested?
Created this PR to allow the tests to run.
Checklist: