Skip to content

Make it more obvious that the cache call does double duty #530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 27, 2022

Conversation

axelson
Copy link
Contributor

@axelson axelson commented Feb 6, 2021

I wasted 20+ minutes because I didn't realize this and couldn't find the information in the documentation anywhere. Hopefully adding it to the Readme will help someone else avoid that pain in the future.

Verified

This commit was signed with the committer’s verified signature.
jaraco Jason R. Coombs
@axelson axelson requested a review from a team as a code owner February 6, 2021 23:40
@ludwigschubert
Copy link

ludwigschubert commented Mar 22, 2021

Same here, but I'd suggest a slight rewording of your note:

sets up a hook to restore the

should probably read sth like

sets up a hook to post (upload) the new version of the cache

I'm including "post" as it seems to be the terminology Github has chosen to use for this.

Thanks for opening a PR, though; I feel this is a simple but important improvement.

@vsvipul vsvipul self-assigned this May 11, 2022
@vsvipul
Copy link
Contributor

vsvipul commented Jun 26, 2022

Our example at https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows should be good enough to make this obvious that the cache needs to be restored before the cached files are required.

Adding @lucascosti to take a look and if a change is required, we should probably add that to the above docs page.

Verified

This commit was signed with the committer’s verified signature.
jaraco Jason R. Coombs
Co-authored-by: Lucas Costi <lucascosti@users.noreply.github.com>
@vsvipul vsvipul requested a review from a team as a code owner June 27, 2022 05:18
Copy link
Contributor

@vsvipul vsvipul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @axelson and @lucascosti for adding this.

@vsvipul vsvipul merged commit baed351 into actions:main Jun 27, 2022
@axelson axelson deleted the document-where-to-cache branch July 2, 2022 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants