Detect cached folders from multiple directories #15
Conversation
it is necessary for the next PR related yarn optimization
dsame
force-pushed
the
project-dir
branch
4 times, most recently
from
001e87e
to
49b76fb
Compare
src/cache-restore.ts
Outdated
| @@ -36,12 +36,12 @@ export const restoreCache = async ( | |||
| ); | |||
| } | |||
|
|
|||
| const primaryKey = `node-cache-${platform}-${packageManager}-${fileHash}`; | |||
| const primaryKey = `node-cache-${platform}-${packageManager}-v2-${fileHash}`; | |||
There was a problem hiding this comment.
Could you please change the key. We do not need to change key because if customers get different cache paths it will resave cache so we can keep cache key as it is.
There was a problem hiding this comment.
@MaksimZhukov what's your opinion?
There was a problem hiding this comment.
I think we can get rid of this, as suggested by Dmitry
src/cache-utils.ts
Outdated
| .filter(fs.existsSync) | ||
| .filter(directory => fs.lstatSync(directory).isDirectory()); |
There was a problem hiding this comment.
@dmitry-shibanov I previously asked to separate these filters for better readability. Is there a reason why we might want them to be combined?
src/cache-utils.ts
Outdated
| // not he wanted, thus we should throw an error not trying to workaround with unexpected | ||
| // result to the whole build | ||
| if (existingDirectories.length === 0) | ||
| throw Error( |
There was a problem hiding this comment.
Could you please change it to warning.
There was a problem hiding this comment.
Besides the motivation in the comment explaining why there should be an error the changing it to waring implies returning empty array that will cause the vague error anyway https://github.com/actions/toolkit/blob/main/packages/cache/src/cache.ts#L27
I need one more confirmation throwing the error should be replaced with warning
dsame
force-pushed
the
project-dir
branch
5 times, most recently
from
2308e96
to
e6a6894
Compare
dsame
force-pushed
the
project-dir
branch
3 times, most recently
from
8951c11
to
f792668
Compare
Description:
A twin for actions#735
Related issue:
Add link to the related issue.
Check list: