IntersectMBO / cardano-base

Code used throughout the Cardano eco-system
Apache License 2.0
93 stars 42 forks source link

CI: Actually use the cache dependencies #464

Closed erikd closed 6 months ago

erikd commented 6 months ago

Previously the GHAs would store the cache after building the dependencies but never restore them. Did a thorough investigation of the issue and added proper comments about how it works and why.

erikd commented 6 months ago

@lehins ledger will be getting similar treatment in the near future.

erikd commented 6 months ago

I don't think this change is needed.

Cache is being restored just fine on current CI builds. The actions/cache action does both saving and restoring, there is no requirement in splitting it into two separate steps. Here is an example of it cahce restoring working: https://github.com/IntersectMBO/cardano-base/actions/runs/8018029965/job/21903084223#step:8:18

So, my question is do we need this duplication, since it already works as expected.

Explicit restore and then explicit save is IMO better than implicit (same action name doing both save and restore). The other reason to accept this PR is to make the GHA in this repo as close as possible to the GHAs in other repos.

In cardano-api the GHA was closely based on the GHA in this repo. After my change to add restoring of the cache before the build, CI build times dropped from about 25 minutes to under 3 minutes.

angerman commented 6 months ago

Having consistent alignment across all repos is already a win in my book.