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

Do not cache node_modules/ #409

Merged
merged 1 commit into from
Mar 13, 2023
Merged

Do not cache node_modules/ #409

merged 1 commit into from
Mar 13, 2023

Conversation

szepeviktor
Copy link
Contributor

It is already handled by actions/setup-node.
Never cache node_modules/. It results in an unpredictable state.
It takes 1-2 seconds to copy files from npm cache to node_modules/.

- name: Setup node v16 and npm cache
uses: actions/setup-node@v3
with:
node-version: 16
cache: 'npm'
cache: npm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the documentation for actions/setup-node they say to have quotes around this value: https://github.com/actions/setup-node#caching-global-packages-data. Not sure if it matters or not but feels like you should follow their guidance

Copy link
Contributor Author

@szepeviktor szepeviktor Mar 13, 2023

Choose a reason for hiding this comment

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

Actually all YAML strings should be quoted: https://github.com/szepeviktor/debian-server-tools/blob/master/webserver/Continuous-integration-Continuous-delivery.yml
I've removed those quotes to be in harmony with other parts of the workflow.

@dkotter dkotter merged commit 40db6b6 into 10up:develop Mar 13, 2023
@jeffpaul jeffpaul added this to the 1.9.0 milestone Mar 13, 2023
@szepeviktor szepeviktor deleted the patch-1 branch March 13, 2023 16:18
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.

3 participants