echobind / bisonapp

A Full Stack Jamstack in-a-box brought to you by Echobind
MIT License
589 stars 29 forks source link

chore: Use setup-node action to cache yarn deps in github actions workflow. #277

Closed cullylarson closed 1 year ago

cullylarson commented 1 year ago

setup-node will cache packages on its own. C.f.:

This PR removes our custom caching and replaces it with setup-node's own cache.

I tested this on Clerestory. "Control" is the old workflow, "Update" is the new workflow.

  Control 1 Control 2 Update 1 Update 2
  103 57 79 79
  66 76 80 110
  68 61 78 90
  71 68 85 77
  68 73 87 108
Average 75.2 67 81.8 92.8

It looks like it's a little slower. Maybe not a beneficial change? It could be that node-setup is doing some extra things that we aren't. But if that doesn't benefit us in terms of runtime, it's probably not worth it. Some pros and cons:

Pros

  1. node-setup will presumably continue to improve. It could also just be GA's inconsistency in runtimes.
  2. It's simpler.

Cons

  1. Potentially slower, but it's hard to tell for sure.
  2. It isn't as explicit as the old solution. I could see someone implementing the old solution again because they think caching is missing. That might not make it past PR review to get into Bison again, but someone who uses it to build a project could easily think that.

Checklist

cball commented 1 year ago

👍 simpler is better here and I agree leaning on something provided by the platform is a good call.