Simplify the Node.js NPM caching example#690
Conversation
|
|
||
| >Note: It is not recommended to cache `node_modules`, as it can break across Node versions and won't work with `npm ci` | ||
|
|
||
| ### macOS and Ubuntu |
There was a problem hiding this comment.
IMO, we should keep this example as well, as it is much more simple without using an extra step. You can instead combine the two headers below (Windows and multiple systems) into one, which will be a much better approach. @bishal-pdMSFT thoughts?
There was a problem hiding this comment.
It's slightly simpler, but this is just copying and pasting anyway. Personally as a user, I just want one example that works everywhere.
More importantly though, this example isn't even 100% correct on Linux & Mac, as the hardcoded path here is actually configurable (by local & global config or env var). Checking npm config get cache is the only way to know what the correct path is for sure on any platform.
Also every other Node package manager in this file (Lerna, Yarn, Yarn 2) already uses a single two-step example like this, instead of having a special case that hardcodes the Mac/Linux default path.
vsvipul
left a comment
vsvipul
left a comment
There was a problem hiding this comment.
Left some comments. Do check.
|
@vsvipul any thoughts on this? |
|
@pimterry Apologies for the very long delay in replying. Let's merge this. Can you please fix the merge conflicts? |
|
No problem @vsvipul - rebased with merge conflicts fixed. |
vsvipul
left a comment
vsvipul
left a comment
There was a problem hiding this comment.
LGTM. Thank you for contributing 🚀
3 participants
This fixes #490. There's no good reason to have 3 examples here, especially when two of them (Windows & 'multiple systems') are effectively the same (they differ only in the ids they give the steps involved). The Linux example was different, but not better for any use cases - it just does the same thing in a more fragile & platform-specific way.
This change simplifies to a single suggested approach (the 3rd existing example) which works immediately for all platforms.