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

Cache node_modules/gems/pods in workflows #3752

Merged
merged 3 commits into from
Jun 29, 2021
Merged

Cache node_modules/gems/pods in workflows #3752

merged 3 commits into from
Jun 29, 2021

Conversation

attaradev
Copy link
Contributor

@attaradev attaradev commented Jun 25, 2021

Details

Cache node_modules in all tasks that install npm packages to speedup workflows

Fixed Issues

Fixes #3627

Tests / QA Steps

  1. Merge this PR
  2. Verify that tests happen as they should, but with caching.

Tested On

n/a – GitHub only.

@attaradev attaradev requested a review from a team as a code owner June 25, 2021 01:30
@MelvinBot MelvinBot requested review from pecanoro and removed request for a team June 25, 2021 01:30
@attaradev
Copy link
Contributor Author

@roryabraham you can have a look, and let me know what you think.
Thanks

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Overall looks good - just the comment about using single quotes instead of double quotes for consistency

.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/platformDeploy.yml Outdated Show resolved Hide resolved
@rushatgabhane
Copy link
Member

I know it's not related to node, but shouldn't we cache ruby gems and CocoaPods too?

Screenshot_20210627-171216

@attaradev
Copy link
Contributor Author

@roryabraham, should I cache the gems and cocoapods?

@attaradev attaradev requested a review from roryabraham June 28, 2021 02:36
@roryabraham
Copy link
Contributor

should I cache the gems and cocoapods

Yes please!

@attaradev attaradev changed the title Cache node_modules in workflows Cache node_modules/gems/pods in workflows Jun 29, 2021
@attaradev
Copy link
Contributor Author

@roryabraham Done

@roryabraham
Copy link
Contributor

@mikeattara What about the gems and pods in the E2E tests?

@attaradev
Copy link
Contributor Author

@roryabraham, I've cached them now. Thought the pods was updating and would not need to be cached

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

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

Okay, @mikeattara I'm approving these changes. But just FYI since we're live-testing them, my plan is to revert this PR if there are any issues. In that case, you'll have to follow-up with another PR to fix any issues.

@roryabraham roryabraham merged commit 1a33442 into Expensify:main Jun 29, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.74-1🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@roryabraham
Copy link
Contributor

@mikeattara I think that this may have caused an error in our last production deploy. Can you please look into that?

@attaradev
Copy link
Contributor Author

@roryabraham looking into it

@attaradev
Copy link
Contributor Author

Can you try re-deploying?

@attaradev
Copy link
Contributor Author

@roryabraham, I think the issue is not with the cache but installing the pods

@roryabraham
Copy link
Contributor

Okay, thanks for investigating. Will keep this on my radar during the next prod deploy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GH Actions] Set up caching for npm installs in all our workflows
4 participants