Skip to content

Conversation

ryanio
Copy link
Contributor

@ryanio ryanio commented Dec 9, 2019

Adds caching of node_modules for faster gh action runs.

See how a cache hit looks here.

Note: we don't have a package-lock.json file in this repository so I am hashing package.json as the cache key instead (also what happens in current .circleci). @alcuadrado likes not having a package-lock.json "as it forces us and the CI to experience the libraries as users would."

@evertonfraga suggested we run a nightly job without caching to catch any potential issues that may happen when an in-range dependency update breaks the build or tests, so I added vm-nightly-test.yml that runs without any caching.

Also accidentally missed running gh actions on pull_request so this PR includes that.

@ryanio ryanio force-pushed the actions-cacheDeps branch from c7c0acb to 4900853 Compare December 9, 2019 18:06
@github-actions
Copy link

github-actions bot commented Dec 9, 2019

Coverage Status

Coverage remained the same at 95.41% when pulling 4900853 on actions-cacheDeps into 0fc97b4 on master.

Copy link
Contributor

@evertonfraga evertonfraga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in the vm-test.yml looks good!

@ryanio would you please add cache to the Coverage and Lint jobs as well?

@ryanio
Copy link
Contributor Author

ryanio commented Dec 13, 2019

oops accidentally committed some residual test files for the PR labeler so that's why the labels got added.

@ryanio would you please add cache to the Coverage and Lint jobs as well?

done!

@ryanio ryanio changed the title github actions: cache dependencies github actions: cache deps, run nightly test Dec 13, 2019
@ryanio ryanio requested a review from evertonfraga December 13, 2019 21:49
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ryan, great work! Also had a short look, change requests from @evertonfraga have been addressed so I think this should be ready for merge.

I think we might now also be ready here to also remove the CircleCI support along one of the next PRs? Will move the requirement checks from Circle to Actions after this post.

@holgerd77 holgerd77 dismissed evertonfraga’s stale review December 15, 2019 09:15

Will dismiss, changes addressed.

@holgerd77 holgerd77 merged commit 845979e into master Dec 15, 2019
@holgerd77 holgerd77 deleted the actions-cacheDeps branch December 15, 2019 09:15
@evertonfraga
Copy link
Contributor

oops accidentally committed some residual test files for the PR labeler so that's why the labels got added.

@ryanio that was me! I created the labels manually, so we have one more item crossed for the migration.

@evertonfraga
Copy link
Contributor

The nightly test successfully ran tonight.

🎉 @ryanio

@holgerd77
Copy link
Member

Nice

@ryanio ryanio mentioned this pull request Dec 16, 2019
3 tasks
@evertonfraga evertonfraga added this to the VM v5 milestone Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants