Conversation
|
Hi, guys. Could someone review my patch, please? |
packages/cache/src/cache.ts
Outdated
| options | ||
| ) | ||
|
|
||
| await listTar(archivePath, compressionMethod) |
There was a problem hiding this comment.
Listing out the contents of the tar file might not be ideal in all scenarios. If users are concerned more about cache speed than log verbosity, we should consider making this optional.
There was a problem hiding this comment.
Thanks for your review. I've added it under if (core.isDebug()) conditinal.
There was a problem hiding this comment.
I'd like to note that @actions/glob also has its own debug output, but it's not the same as listTar:
|
Ping |
|
@joshmgross I've updated the PR a week ago. Do you expect from me any other actions or do I miss something? |
|
@aiqiaoy or @yacaovsnc could you take a look at this PR? |
|
Thanks @rosik for the PR. Could you please add a test for the |
856f207 to
967b069
Compare
|
I've added a unit test, but I'm worried it's not covered by an end-to-end test. The |
|
Sorry for the silence, I was out for the last couple weeks. Personally I think it's fine that this isn't covered in the end-to-end test - to test that we need to set FWIW, I forked the repo and set |
yacaovsnc
left a comment
There was a problem hiding this comment.
Thanks for the PR. Looks good to me.
- Print cache size when saving cache similarly to restoring - Print restore success similarly to saving - Print cached file list if debug logging is enabled See also: actions/cache#471
|
If you ask for my opinion, I vote for enabling ACTIONS_STEP_DEBUG in the main repo. It sounds helpful for the coverage. So, what's next? I've rebased my patch on top of the fresh main branch. Do you expect any other actions from me now? |
|
Ping |
|
@rosik the cache action can include this change after we release a new npm package. Are you waiting for this change? @konradpabjan let's include this change next time we release. Do you handle npm releases for cache toolkit too? |
So so. We can always invent a workaround for debugging, but it's usually a little bit nasty. |
This makes logs of saving and restoring cache more similar to each other.
Part of actions/cache#471