-
Notifications
You must be signed in to change notification settings - Fork 219
Conversation
Size Change: +48.1 kB (+5%) 🔍 Total Size: 924 kB
ℹ️ View Unchanged
|
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @alexflorisca! Thank you for this great PR! We indeed needed faster CI checks! 🙌
I've left some change requests below! 👍
Thanks for the review @tarhi-saad! I've addressed your comments above 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @alexflorisca! Thank you for answering all my questions! This LGTM, so let's 🚢 it!
This PR caches
node_modules
in between github action workflows to speed things up. The cache is based on thepackage-lock.json
file and the os that the action is running on. So we will be invalidating the cache everytime a dependency changes. This is a significant time saver, as wenpm ci
in quite a few places independently which is unnecessary.Note that we are already using
actions/setup-node
with thecache: npm
option. This caches the~/.npm
directory which contains package data to save downloading from servers for everynpm install
. However node_modules are not cached as part of theactions/setup-node
action.Also, for the Unit Tests workflow, we are running the build step only in the Setup job and saving as artefacts for the other two jobs
Fixes #5689
To test: