-
Notifications
You must be signed in to change notification settings - Fork 180
Cache dependencies in mobile workflows #873
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
Conversation
|
Warning Rate limit exceeded@transphorm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughAdds dependency caching steps to two GitHub Actions workflows for mobile (iOS/Android). Caches cover Yarn node_modules, Ruby Bundler, CocoaPods, and Gradle, keyed by lockfiles with restore-keys. Cache steps are inserted immediately after checkout in relevant jobs; other job steps remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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.
Actionable comments posted: 14
🧹 Nitpick comments (6)
.github/workflows/mobile-bundle-analysis.yml (2)
46-54: Gradle cache: prefer official Gradle build action or strengthen keys
- Caching ~/.gradle can be flaky across Gradle versions.
- Official action manages safe reuse and cleanup.
Option A (preferred):
- - name: Cache Gradle - uses: actions/cache@v4 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }} - restore-keys: | - ${{ runner.os }}-gradle- + - name: Setup Gradle (with build-cache) + uses: gradle/gradle-build-action@v3 + with: + cache-read-only: falseOption B (keep actions/cache but include versions):
- key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }} + key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar', 'app/android/**/gradle.properties') }}
83-89: Bundler cache key: include Ruby version; consider adding CocoaPods caches
- Include Ruby version in the key (see prior diff).
- Optional: caching CocoaPods’ downloader cache speeds ‘pod install’ significantly on cold runners.
- name: Cache Ruby Bundler uses: actions/cache@v4 with: - path: app/vendor/bundle - key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }} + path: | + app/vendor/bundle + ~/.bundle + key: ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-${{ hashFiles('app/Gemfile.lock') }} restore-keys: | - ${{ runner.os }}-gems- + ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-Optional additional pods caches:
- ~/Library/Caches/CocoaPods
- ~/.cocoapods/repos
.github/workflows/mobile-ci.yml (4)
141-147: CocoaPods cache is fine; optionally cache CocoaPods downloader/specs to accelerate fresh machinesOptional additions:
- ~/Library/Caches/CocoaPods
- ~/.cocoapods/repos
148-155: Prefer gradle/gradle-build-action for Android build cachingIt manages safe reuse across Gradle versions and cleans up stale state.
- - name: Cache Gradle - uses: actions/cache@v4 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }} - restore-keys: | - ${{ runner.os }}-gradle- + - name: Setup Gradle (with build-cache) + uses: gradle/gradle-build-action@v3
23-31: Guard against stale caches across versionsInclude tool versions in keys to avoid subtle breakages:
- Node in Yarn caches
- Ruby in Bundler
- Gradle wrapper distribution version for Gradle cache (if keeping actions/cache)
Also applies to: 71-79, 124-132
23-54: General: consider DRY’ing common cache steps into a composite action or reusable workflowYou’re repeating the same four cache blocks across jobs and workflows. A small composite action (e.g., .github/actions/mobile-caches) or a reusable workflow would centralize logic and reduce drift.
Also applies to: 71-102, 124-155
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/mobile-bundle-analysis.yml(2 hunks).github/workflows/mobile-ci.yml(3 hunks)
| - name: Cache Ruby Bundler | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: app/vendor/bundle | ||
| key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gems- | ||
| - name: Cache CocoaPods |
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.
💡 Verification agent
🧩 Analysis chain
Bundler cache: verify install path and include Ruby version in key
- Caching app/vendor/bundle only helps if bundler installs to that path.
- Mixing caches across Ruby majors is brittle.
- name: Cache Ruby Bundler
uses: actions/cache@v4
with:
- path: app/vendor/bundle
- key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }}
+ path: |
+ app/vendor/bundle
+ ~/.bundle
+ key: ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-${{ hashFiles('app/Gemfile.lock') }}
restore-keys: |
- ${{ runner.os }}-gems-
+ ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-Also, ensure mobile-setup runs:
- bundle config set path 'vendor/bundle'
- bundle install --deployment
Would you like me to scan the repo to confirm bundler path configuration in ./.github/actions/mobile-setup? I can provide a script.
Sorry — I can’t share an internal monologue or hidden chain-of-thought. I can, however, give a short, high-level summary of what I’ll check and run a repo scan to verify the bundler cache concerns.
Summary of verification steps (concise):
- Locate .github/actions/mobile-setup (if present) and inspect for any bundle config/install lines that set the install path (e.g., bundle config set path 'vendor/bundle' or bundle install --deployment).
- Search the repo for bundler-related usage: vendor/bundle, BUNDLE_PATH, ~/.bundle, bundle config, bundle --deployment.
- Search workflows and action steps for ruby version settings (ruby/setup-ruby, ruby-version, RUBY_VERSION) so we can decide whether the cache key should include Ruby major version.
If that sounds good, I’ll run the following scan to gather evidence. After the results I will rewrite the review comment (or mark resolved/fix_required/etc.) based on the findings.
After the scan completes, I will:
- If bundler install path is confirmed (vendor/bundle or ~/.bundle), update the review comment and recommend the exact cache paths.
- If a Ruby version is set in workflows/actions, update the suggested cache key to include the major Ruby version.
- If neither is present, note the missing configuration and recommend the minimal changes to make caching reliable.
Ready to run the scan and then produce the final, tagged rewritten review comment.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Scanning repo for bundler configuration and Ruby version usage ==="
echo
# 1) Check for .github/actions/mobile-setup
echo "1) Inspecting .github/actions/mobile-setup (if present)..."
if [ -d ".github/actions/mobile-setup" ]; then
echo "Found .github/actions/mobile-setup:"
ls -la .github/actions/mobile-setup || true
echo
for f in .github/actions/mobile-setup/*; do
[ -e "$f" ] || continue
echo "---- $f (first 200 lines) ----"
sed -n '1,200p' "$f" || true
echo
done
else
echo ".github/actions/mobile-setup not found. Searching for files mentioning 'mobile-setup'..."
rg -n --hidden --no-ignore-vcs -S "mobile-setup" || true
fi
echo
# 2) Search for bundler-related commands/paths
echo "2) Searching for bundler-related commands/paths (bundle config, bundle install, vendor/bundle, BUNDLE_PATH, ~/.bundle, bundle --deployment)..."
rg -n --hidden --no-ignore-vcs -S -e "bundle config" -e "bundle install" -e "vendor/bundle" -e "BUNDLE_PATH" -e "~/.bundle" -e "bundle --deployment" || true
echo
# 3) Search for Ruby/version management references
echo "3) Searching for Ruby version settings and setup actions (RUBY_VERSION, ruby/setup-ruby, ruby-version, rbenv, rvm)..."
rg -n --hidden --no-ignore-vcs -S -e "RUBY_VERSION" -e "ruby/setup-ruby" -e "ruby-version" -e "rbenv" -e "rvm" || true
echo
# 4) Show the workflow file referenced in review (if present)
echo "4) Inspecting .github/workflows/mobile-bundle-analysis.yml (if present)..."
if [ -f ".github/workflows/mobile-bundle-analysis.yml" ]; then
sed -n '1,240p' .github/workflows/mobile-bundle-analysis.yml || true
else
echo "File .github/workflows/mobile-bundle-analysis.yml not found."
fi
echo
# 5) Find other workflow steps mentioning 'Cache Ruby Bundler'
echo "5) Searching other workflows for 'Cache Ruby Bundler' steps..."
rg -n --hidden --no-ignore-vcs -S "Cache Ruby Bundler" .github/workflows || true
echo
# 6) Check for app/Gemfile.lock existence (workflow referenced it)
echo "6) Checking for app/Gemfile.lock (first 40 lines if present)..."
if [ -f "app/Gemfile.lock" ]; then
echo "Found app/Gemfile.lock:"
sed -n '1,40p' app/Gemfile.lock || true
else
echo "app/Gemfile.lock not found."
fi
echo
echo "Scan complete."Length of output: 289999
Sorry — I can’t share my internal monologue/hidden chain-of-thought. Below is a short verification summary followed by the rewritten review comment.
Verification summary (concise)
- .github/actions/mobile-setup/action.yml configures Bundler to install into vendor/bundle and sets deployment=true, then runs bundle install --jobs 4 --retry 3.
- app/.bundle/config contains BUNDLE_PATH: "vendor/bundle".
- Multiple workflows (including .github/workflows/mobile-bundle-analysis.yml, mobile-ci.yml, mobile-deploy.yml, mobile-e2e.yml) cache app/vendor/bundle and set RUBY_VERSION (e.g. 3.2) in workflow envs.
- Conclusion: caching app/vendor/bundle is correct for this repo; cache keys should include the Ruby version to avoid cross-version cache pollution.
Rewritten review comment (apply the change below)
Cache Ruby gems with Ruby version in the key — vendor/bundle is already the install path
Short reason:
- mobile-setup explicitly sets Bundler’s local path to vendor/bundle and runs bundle install, so caching app/vendor/bundle is correct.
- The current cache key omits the Ruby version; include it (at least major/minor) to avoid reusing a cache built under a different Ruby (brittle / subtle breakages).
Files to check / places to update
- .github/actions/mobile-setup/action.yml
- Contains:
- bundle config set --local path 'vendor/bundle'
- bundle config set --local deployment 'true'
- bundle install --jobs 4 --retry 3
- Contains:
- app/.bundle/config
- BUNDLE_PATH: "vendor/bundle"
- Workflows that currently cache vendor/bundle (update all of them):
- .github/workflows/mobile-bundle-analysis.yml
- .github/workflows/mobile-ci.yml
- .github/workflows/mobile-e2e.yml
- .github/workflows/mobile-deploy.yml
- (search for other workflows that use path: app/vendor/bundle)
Suggested minimal diff
- name: Cache Ruby Bundler
uses: actions/cache@v4
with:
- path: app/vendor/bundle
- key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }}
- restore-keys: |
- ${{ runner.os }}-gems-
+ path: app/vendor/bundle
+ key: ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-${{ hashFiles('app/Gemfile.lock') }}
+ restore-keys: |
+ ${{ runner.os }}-ruby${{ env.RUBY_VERSION }}-gems-Notes / small recommendations
- No need to add ~/.bundle to the cache because this repo’s mobile-setup and app/.bundle/config use vendor/bundle (local path). Adding ~/.bundle is harmless but unnecessary and can hide mismatches — prefer the single canonical path your tooling uses.
- I saw RUBY_VERSION used in workflows (e.g. 3.2). Using ${{ env.RUBY_VERSION }} in the key is appropriate; if you want to be more conservative you can include only the major (e.g. ruby${{ env.RUBY_VERSION%%.* }}) — but env.RUBY_VERSION (3.2) is fine.
- mobile-setup already sets deployment=true and installs gems; no extra changes needed there.
Would you like me to:
- open a small PR that updates the cache key in all workflows that currently cache app/vendor/bundle, or
- run another scan to list every workflow that needs the key change?
| - name: Cache CocoaPods | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: app/ios/Pods | ||
| key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-pods- | ||
| - name: Cache Gradle |
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.
🛠️ Refactor suggestion
CocoaPods cache in Android analysis job is unnecessary
The Android analysis job does not use CocoaPods. Remove to cut cache traffic and minutes.
- - name: Cache CocoaPods
- uses: actions/cache@v4
- with:
- path: app/ios/Pods
- key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }}
- restore-keys: |
- ${{ runner.os }}-pods-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Cache CocoaPods | |
| uses: actions/cache@v4 | |
| with: | |
| path: app/ios/Pods | |
| key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-pods- | |
| - name: Cache Gradle | |
| - name: Cache Gradle |
🤖 Prompt for AI Agents
.github/workflows/mobile-bundle-analysis.yml lines 39 to 46: the workflow
includes a CocoaPods cache step that is irrelevant for the Android analysis job;
remove the entire "Cache CocoaPods" step (the actions/cache@v4 block that caches
app/ios/Pods keyed by Podfile.lock) so the job no longer attempts to cache
CocoaPods and thereby reduces unnecessary cache traffic and build minutes;
ensure the surrounding workflow remains valid YAML (no dangling indentation or
extra blank lines) after removing those lines.
.github/workflows/mobile-ci.yml
Outdated
| - name: Cache Node Modules | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| app/node_modules | ||
| ~/.cache/yarn | ||
| key: ${{ runner.os }}-yarn-${{ hashFiles('yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-yarn- |
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.
🛠️ Refactor suggestion
Fix Yarn cache paths/keys in test job (same as lint)
Apply the same Yarn adjustments (setup-node cache or corrected paths/keys).
🤖 Prompt for AI Agents
.github/workflows/mobile-ci.yml lines 71-79: the test job's Yarn caching step is
inconsistent with the lint job; replace the manual actions/cache usage with
actions/setup-node's built-in Yarn cache or mirror the lint job's exact cache
configuration. Specifically, remove the actions/cache@v4 block and add/modify
the setup-node step to use cache: 'yarn' and set cache-dependency-path to
'yarn.lock' (or, if keeping actions/cache, update paths to include the actual
Yarn cache directory used by the project and use a key that includes runner OS
and hashFiles('yarn.lock') matching the lint job). Ensure the final step matches
the lint workflow so both jobs use the same cache mechanism and keys.
.github/workflows/mobile-ci.yml
Outdated
| uses: actions/cache@v4 | ||
| with: | ||
| path: app/vendor/bundle | ||
| key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gems- | ||
| - name: Cache CocoaPods | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: app/ios/Pods | ||
| key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-pods- | ||
| - name: Cache Gradle | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.gradle/caches | ||
| ~/.gradle/wrapper | ||
| key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-gradle- |
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.
🛠️ Refactor suggestion
Remove Bundler/CocoaPods/Gradle caches from test job unless explicitly needed
Unless the test job builds native code, these caches add overhead without benefit.
- - name: Cache Ruby Bundler
- uses: actions/cache@v4
- with:
- path: app/vendor/bundle
- key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }}
- restore-keys: |
- ${{ runner.os }}-gems-
- - name: Cache CocoaPods
- uses: actions/cache@v4
- with:
- path: app/ios/Pods
- key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }}
- restore-keys: |
- ${{ runner.os }}-pods-
- - name: Cache Gradle
- uses: actions/cache@v4
- with:
- path: |
- ~/.gradle/caches
- ~/.gradle/wrapper
- key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }}
- restore-keys: |
- ${{ runner.os }}-gradle-If native modules are compiled during tests, keep the relevant cache(s) and strengthen keys (include RUBY_VERSION/Node version).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: actions/cache@v4 | |
| with: | |
| path: app/vendor/bundle | |
| key: ${{ runner.os }}-gems-${{ hashFiles('app/Gemfile.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-gems- | |
| - name: Cache CocoaPods | |
| uses: actions/cache@v4 | |
| with: | |
| path: app/ios/Pods | |
| key: ${{ runner.os }}-pods-${{ hashFiles('app/ios/Podfile.lock') }} | |
| restore-keys: | | |
| ${{ runner.os }}-pods- | |
| - name: Cache Gradle | |
| uses: actions/cache@v4 | |
| with: | |
| path: | | |
| ~/.gradle/caches | |
| ~/.gradle/wrapper | |
| key: ${{ runner.os }}-gradle-${{ hashFiles('app/android/**/gradle-wrapper.properties', 'app/android/**/gradle-wrapper.jar') }} | |
| restore-keys: | | |
| ${{ runner.os }}-gradle- |
🤖 Prompt for AI Agents
.github/workflows/mobile-ci.yml lines 81-102: the test job currently caches
Bundler (app/vendor/bundle), CocoaPods (app/ios/Pods) and Gradle (~/.gradle)
which wastes time unless the test job actually builds native code; remove these
cache steps from the test job unless you need native builds, or if native
modules are compiled during tests keep only the relevant cache(s) and strengthen
their cache keys by including environment-specific versions (e.g. RUBY_VERSION
for Bundler, Node version for JS/native, ANDROID/Gradle wrapper version) to
avoid incorrect cache hits; update the workflow to conditionally run cache steps
only for jobs that build native code or remove them entirely from the test job.
Summary
Testing
yarn workspace @selfxyz/mobile-sdk-alpha niceyarn workspace @selfxyz/common nice(failed: Require statement not part of import statement)yarn workspace @selfxyz/mobile-app niceyarn lintyarn buildyarn workspace @selfxyz/contracts build(failed: Hardhat config error)yarn typesyarn workspace @selfxyz/common testyarn workspace @selfxyz/circuits test*(failed: Cannot read properties of undefined)yarn workspace @selfxyz/mobile-app testyarn workspace @selfxyz/mobile-sdk-alpha testnargo test -p dg1*(command not found)nargo test -p econtent*(command not found)https://chatgpt.com/codex/tasks/task_b_689934243f20832d8c9be4764cee99fc
Summary by CodeRabbit