Extend cache action - Githubissues

julia-actions / cache

A shortcut action to cache Julia artifacts, packages, and registries.

MIT License

38 stars 9 forks source link

Extend cache action #4

Closed rikhuijzer closed 2 years ago

rikhuijzer commented 2 years ago

Closes #2.

rikhuijzer commented 2 years ago

The current failures are because the cache-name which is passed from test-save to test-restore is not always passed to the same operating system, as figured out by manually checking the keys and noticing that the Ubuntu save key is used in the Windows restore steps. Then, the restore key doesn't match anymore so it is an unexpected cache miss.

rikhuijzer commented 2 years ago

Great. Everything works again and is now tested on all three operating systems.

I've considered testing the PATHS logic more, but chose not too for now. It is already covered pretty well since currently two things are tested:

artifacts is inside PATHS (otherwise, the artifacts will not be cached and the last job in CI.yml will fail)
the PATHS newlines are properly propagated into cache.key (otherwise, nothing will be cached at all and the last job in CI.yml will fail)

rikhuijzer commented 2 years ago

Should we rename this to julia-actions/cache then?

Maybe, cache-julia to be in line with setup-julia, julia-buildpkg, julia-runtest etc? I find the names a bit double so just cache makes more sense indeed. Maybe to avoid clashing fork names?

Good idea to rename. I'll leave the decision up to you to what exactly. I'm okay with cache, cache-julia and similar names.

SaschaMann commented 2 years ago

cache-julia sounds like it caches Julia which it doesn't (and probably shouldn't), I'll go with just cache then

rikhuijzer commented 2 years ago

cache-julia sounds like it caches Julia which it doesn't (and probably shouldn't), I'll go with just cache then

Fair enough :+1:

I'm seeing some possible discussion points in the README too. Can we merge this first to avoid having a too long PR? Then, I'll open a new PR for the README

SaschaMann commented 2 years ago

I'm seeing some possible discussion points in the README too. Can we merge this first to avoid having a too long PR? Then, I'll open a new PR for the README

Yea that's fine. I'll tag a new release once the README has been updated then :)

rikhuijzer commented 2 years ago

Thank you for that and also for the reviews :smile: I've learned some things from them

I'll do the README tomorrow.