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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: using built-in caching from setup-node #1501

Merged
merged 1 commit into from Jul 20, 2021

Conversation

dlockhart
Copy link
Member

Now that this is built into the setup-node action, seems like we can save some code by using it. Not sure whether no longer skipping the NPM install step will make things super slow, but I'll test that out.

@dlockhart
Copy link
Member Author

The installation step (which now always runs) seems to take between 10-20 seconds when there's a cache hit, compared to being skipped entirely before. That seems like an OK trade-off to me to reduce the size & complexity of the workflows.

@BrightspaceUI BrightspaceUI deleted a comment from github-actions bot Jul 20, 2021
@svanherk
Copy link
Contributor

Action will try to search package-lock.json or yarn.lock (npm 7.x supports yarn.lock files) files in the repository root and throw error if no one is found. The hash of found file will be used as cache key (the same approach like actions/cache recommends).

If I'm understanding the above correctly (from the caching PR), the cache busting works the same as what we had before?

@dlockhart
Copy link
Member Author

Yes, it's actually slightly better in that they're putting the environment in the cache key as well.

The main difference is that they're not surfacing whether or not there was a cache hit, so we can no longer skip steps based on that. Which they don't seem to think is a problem, and doesn't seem to add significant time to our builds from my testing. If anything it lets us combine a few of our "install" steps into one, which I've done here.

@svanherk
Copy link
Contributor

Yeah, I agree the cleanup of the workflows is worth it, much nicer. We can keep an eye on things to see if the install times get worse over time, but ideally GitHub Actions gives us the ability to retrigger individual jobs. Then it matters even less, if I can retrigger the Sauce job and not all the others.

@dlockhart dlockhart merged commit 8e76d8f into main Jul 20, 2021
@dlockhart dlockhart deleted the dlockhart/setup-node-cache branch July 20, 2021 15:10
@ghost
Copy link

ghost commented Jul 20, 2021

🎉 This PR is included in version 1.152.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants