[No QA] Use setup-node and setup-ruby for caching and installs#9815
Merged
Conversation
roryabraham
changed the title
roryabraham
commented
Jul 9, 2022
| - uses: actions/setup-node@09ba51f18e18a3756fea1f54d09c6745c064491d | ||
| with: | ||
| node-version: 16 | ||
| cache: npm |
Contributor
Author
There was a problem hiding this comment.
Note the use of built-in caching as opposed to actions/cache. Since this is maintained by GitHub hopefully it will work better on average than our caching has in the past. Also it's easier / less configuration needed.
roryabraham
commented
Jul 9, 2022
| - uses: Expensify/App/.github/actions/composite/setupNode@main | ||
|
|
||
| - uses: actions/setup-ruby@v1 | ||
| - uses: ruby/setup-ruby@08245253a76fa4d1e459b7809579c62bd9eb718a |
Contributor
Author
There was a problem hiding this comment.
Note the use of https://github.com/ruby/setup-ruby instead of deprecated https://github.com/actions/setup-ruby.
roryabraham
commented
Jul 9, 2022
| timeout_minutes: 10 | ||
| max_attempts: 5 | ||
| command: npm ci | ||
| bundler-cache: true |
Contributor
Author
There was a problem hiding this comment.
This flag runs 'bundle install' and caches installed gems automatically
roryabraham
commented
Jul 9, 2022
|
|
||
| - name: Install bundler | ||
| run: | | ||
| bundle config path vendor/bundle |
Contributor
Author
There was a problem hiding this comment.
From the ruby/setup-ruby docs:
To perform caching, this action will use bundle config --local path $PWD/vendor/bundle.
Therefore, the Bundler path should not be changed in your workflow for the cache to work (no bundle config path).
Since the action already used vendor/bundle for the path, we don't need to worry about setting it before installing bundler.
Contributor
Author
|
Checks are failing because the
|
AndrewGable
previously approved these changes
Jul 12, 2022
Justicea83
suggested changes
Jul 12, 2022
Justicea83
left a comment
Justicea83
left a comment
Contributor
There was a problem hiding this comment.
Everything looks good if we fix the conflicts
Contributor
|
It seems we have some linting issues, can we fix it and get it merged ASAP? |
Contributor
Author
|
Tests are going to fail for the reason described in this comment, so I'm running them locally for good measure: Jest Unit TestsClick to expand!ESLintNOTE: The only failing files here are ones that only exist locally and are not committed to this repo (I have them in my global Validate GitHub ActionsVerify Podfile
|
AndrewGable
approved these changes
Jul 12, 2022
|
@AndrewGable looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
Contributor
|
@roryabraham mentioned why here, but these can only be tested on |
95 tasks
Contributor
|
🚀 Deployed to production by @chiragsalian in version: 1.1.84-13 🚀
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Fixed Issues
$ n/a
Tests