search

prettier / eslint-config-prettier

Turns off all rules that are unnecessary or might conflict with Prettier.
MIT License
5.39k stars 255 forks source link

ci(workflow): add cache to workflows using actions/setup-node #195

Closed oscard0m closed 2 years ago

oscard0m commented 3 years ago

Description

Add cache to workflows using actions/setup-node

Context

setup-node GitHub Action just released a new option to add cache to steps using it.

You can find the details here: https://github.blog/changelog/2021-07-02-github-actions-setup-node-now-supports-dependency-caching/

🤖 This PR has been generated automatically by this octoherd script, feel free to run it in your GitHub user/org repositories! 💪🏾

oscard0m commented 3 years ago

Since actions/cache is using node_modules and not a local ~/.npm folder I'm not sure if this PR have sense: https://github.com/actions/setup-node/issues/286

lydell commented 3 years ago

Hi! Thanks for the pull request!

This is all new to me, and I haven’t really had time to play around with the new caching stuff. But this documentation suggests that one shouldn’t cache node_modules: https://github.com/actions/cache/blob/main/examples.md#node---npm

oscard0m commented 3 years ago

Hi! Thanks for the pull request!

This is all new to me, and I haven’t really had time to play around with the new caching stuff. But this documentation suggests that one shouldn’t cache node_modules: https://github.com/actions/cache/blob/main/examples.md#node---npm

Should we try to remove this actions/cache step then @lydell ?

lydell commented 3 years ago

Should we try to remove this actions/cache step then @lydell ?

Yes, feel free to experiment! Would be interesting to see if there’s any time difference, and if npm ci --offline succeeds when there is a cache (to verify that the cache helps against npm outage).

lydell commented 2 years ago

Status: IMO the current setup works just fine, and I don’t really understand the new way of caching.

Do you still feel like experimenting with this? Otherwise I think we’re going to close this one (unless you convince me that the new caching approach is better.)

lydell commented 2 years ago

Closing because no response.

oscard0m commented 2 years ago

Sorry for the delay on this @lydell, following up here: https://github.com/prettier/eslint-config-prettier/pull/210