You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We can set the key for caches to include all three sets of package.json and package-lock.json.
That way, either no caches will hit, or all caches will hit.
Should save our sanity trying to debug partial cache restores.
This is also a performance optimization, as it will save us from spending the time doing partial restores, which then immediately get wiped by npm ci during bootstrap.
I want to stress that, given the current state of master, this is a no-downsides fix. I want to do this for master.
We can keep investigating #33 in spare time. I don't support the initial premise of that PR, but I'm willing to be proved wrong on that, which is why I'd still try to think if there are ways it could work. And obviously other things are a higher priority. This issue is for a simple correctness fix and performance improvement and should not be controversial to do while #33 remains an experiment. The merge conflicts from doing this would be trivially easy to fix and I volunteer to do them myself.
The text was updated successfully, but these errors were encountered:
We can set the
key
for caches to include all three sets ofpackage.json
andpackage-lock.json
.That way, either no caches will hit, or all caches will hit.
Should save our sanity trying to debug partial cache restores.
This is also a performance optimization, as it will save us from spending the time doing partial restores, which then immediately get wiped by
npm ci
during bootstrap.I want to stress that, given the current state of
master
, this is a no-downsides fix. I want to do this formaster
.We can keep investigating #33 in spare time. I don't support the initial premise of that PR, but I'm willing to be proved wrong on that, which is why I'd still try to think if there are ways it could work. And obviously other things are a higher priority. This issue is for a simple correctness fix and performance improvement and should not be controversial to do while #33 remains an experiment. The merge conflicts from doing this would be trivially easy to fix and I volunteer to do them myself.
The text was updated successfully, but these errors were encountered: