Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added yarn cache to CI workflows #13716

Merged
merged 1 commit into from
Nov 30, 2021
Merged

Added yarn cache to CI workflows #13716

merged 1 commit into from
Nov 30, 2021

Conversation

oscard0m
Copy link
Contributor

@oscard0m oscard0m commented Nov 6, 2021

Description

Add cache to workflows using actions/setup-node

Context

setup-node GitHub Action just released a new option to add cache to steps using it.

You can find the details here: https://github.blog/changelog/2021-07-02-github-actions-setup-node-now-supports-dependency-caching/

Re-generated PR from #13597


馃 This PR has been generated automatically by this octoherd script, feel free to run it in your GitHub user/org repositories! 馃挭馃従

@oscard0m
Copy link
Contributor Author

oscard0m commented Nov 6, 2021

@daniellockyer I did a cleanup of my forks and I had to re-generate the fork, PR's and so (not sure if there was a way to link everything). Closing old PR

@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2021

Codecov Report

Merging #13716 (2712377) into main (59f6521) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #13716      +/-   ##
==========================================
- Coverage   56.28%   56.27%   -0.01%     
==========================================
  Files         552      552              
  Lines       46028    46028              
  Branches     4127     4127              
==========================================
- Hits        25906    25902       -4     
- Misses      20078    20083       +5     
+ Partials       44       43       -1     
Impacted Files Coverage 螖
core/server/models/base/plugins/events.js 69.48% <0.00%> (-1.48%) 猬囷笍
core/shared/config/loader.js 97.36% <0.00%> (酶)
core/shared/settings-cache/cache.js 69.44% <0.00%> (酶)
...ore/frontend/services/routing/helpers/templates.js 68.06% <0.00%> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 59f6521...2712377. Read the comment docs.

@daniellockyer
Copy link
Member

@oscard0m Thanks for the updated PR! It looks good to me the the setup-node actions seems to fail upon the job finishing - do you know why?

@daniellockyer
Copy link
Member

actions/setup-node#317 (comment) 馃く

@oscard0m
Copy link
Contributor Author

oscard0m commented Nov 21, 2021

actions/setup-node#317 (comment) 馃く

OMG! Do you plan to block this until that issue is addressed? @daniellockyer

@daniellockyer daniellockyer self-assigned this Nov 23, 2021
daniellockyer added a commit that referenced this pull request Nov 25, 2021
refs #13716
refs actions/setup-node#317 (comment)

- the `setup-node` GitHub Action seems to use a shell command to get the
  cache path, but these are colorised when `FORCE_COLOR` is enabled
- this causes the Action to fail to read the path correctly
- the comment referenced above suggests to remove `FORCE_COLOR` but it's
  nice to have colored output for our tests
- instead, I'm disabling the environment variable on the `setup-node`
  action so it still works
- I've tested with the referenced PR and this unblocks dependency caching 馃帀
@daniellockyer
Copy link
Member

@oscard0m I've implemented a workaround so we didn't have to disable FORCE_COLOR for the whole workflow. I've tested with your PR on top and caching seems to work!

If you'd just like to give it a quick rebase + then we should be good to merge 馃檪

@oscard0m
Copy link
Contributor Author

@oscard0m I've implemented a workaround so we didn't have to disable FORCE_COLOR for the whole workflow. I've tested with your PR on top and caching seems to work!

If you'd just like to give it a quick rebase + then we should be good to merge 馃檪

Thanks @daniellockyer . Rebased pushed. Let's see what the CI says now 馃嵖

@daniellockyer daniellockyer changed the title ci(workflow): add cache 'yarn' to workflows Added yarn cache to CI workflows Nov 30, 2021
@daniellockyer daniellockyer merged commit 141f794 into TryGhost:main Nov 30, 2021
@daniellockyer
Copy link
Member

@oscard0m Thanks for the PR! 馃暫馃徎

@oscard0m oscard0m deleted the add-cache-to-node-workflows branch December 9, 2021 18:52
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

3 participants