refine mobile deploy auto pr logic #1234
Conversation
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1).github/workflows/**/*.{yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🪛 actionlint (1.7.7).github/workflows/mobile-deploy.yml1058-1058: property "version" is not defined in object type {} (expression) ⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/fastlane/Fastfile (1)
104-105: Consider extracting the duplicate Node.js semver script.The same Node.js semver bump script appears in both iOS and Android deployment paths. Consider extracting this to a helper method to reduce duplication and improve maintainability.
Add a helper method in
helpers/version_manager.rb:module Fastlane module Helpers module VersionManager def self.bump_package_version(version_bump) sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") end end end endThen replace both occurrences with:
- sh("cd .. && node -e \"const fs = require('fs'); const pkg = require('./package.json'); const semver = require('semver'); pkg.version = semver.inc(pkg.version, '#{version_bump}'); fs.writeFileSync('./package.json', JSON.stringify(pkg, null, 2) + '\\n');\"") + Fastlane::Helpers::VersionManager.bump_package_version(version_bump)Also applies to: 307-308
.github/actions/create-version-bump-pr/action.yml (1)
44-44: Add error handling for the cherry-pick operation.The
git cherry-pickcommand can fail if there are conflicts between the temporary commit and the dev branch. Consider adding error handling or at least a clear error message to help diagnose failures.Apply this diff to add error handling:
# Cherry-pick only the version changes - git cherry-pick temp-version-commit + if ! git cherry-pick temp-version-commit; then + echo "❌ Error: Failed to cherry-pick version changes. This may indicate conflicts between staging and dev." + echo "Please manually resolve conflicts and create the version bump PR." + git cherry-pick --abort + exit 1 + fi.github/workflows/mobile-deploy.yml (1)
155-173: Consider extracting version bump determination to a reusable action.The version bump determination logic is duplicated between iOS and Android jobs. Consider extracting this to a composite action under
./.github/actions/determine-version-bumpto reduce duplication.As per coding guidelines for GitHub workflows to use composite actions for common patterns.
Create
.github/actions/determine-version-bump/action.yml:name: Determine Version Bump description: Determines version bump type from PR labels or workflow input inputs: workflow_input: description: Version bump from workflow_dispatch input required: false default: 'build' pr_labels: description: JSON array of PR labels required: false default: '[]' event_name: description: GitHub event name required: true outputs: version_bump: description: Determined version bump type value: ${{ steps.determine.outputs.version_bump }} runs: using: composite steps: - name: Determine version bump id: determine shell: bash run: | VERSION_BUMP="${{ inputs.workflow_input }}" if [ "${{ inputs.event_name }}" = "pull_request" ]; then LABELS='${{ inputs.pr_labels }}' if echo "$LABELS" | grep -q "version:major"; then VERSION_BUMP="major" elif echo "$LABELS" | grep -q "version:minor"; then VERSION_BUMP="minor" elif echo "$LABELS" | grep -q "version:patch"; then VERSION_BUMP="patch" fi fi echo "version_bump=$VERSION_BUMP" >> $GITHUB_OUTPUT echo "📦 Version bump type: $VERSION_BUMP"Then replace both occurrences in the workflow with:
- name: Determine version bump from PR labels or input id: version-bump uses: ./.github/actions/determine-version-bump with: workflow_input: ${{ inputs.version_bump || 'build' }} pr_labels: ${{ toJSON(github.event.pull_request.labels.*.name) }} event_name: ${{ github.event_name }}Also applies to: 746-764
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/create-version-bump-pr/action.yml(1 hunks).github/workflows/mobile-deploy.yml(5 hunks)app/fastlane/Fastfile(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows
Files:
.github/workflows/mobile-deploy.yml
🪛 actionlint (1.7.7)
.github/workflows/mobile-deploy.yml
1023-1023: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: build-deps
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (4)
.github/workflows/mobile-deploy.yml (4)
155-173: LGTM! Version bump determination logic is clear and correct.The logic properly handles both workflow_dispatch inputs and PR label overrides. The output is correctly set for downstream steps to consume.
746-764: LGTM! Consistent with the iOS version bump logic.The Android version bump determination follows the same pattern as iOS, ensuring consistency across platforms.
1054-1066: LGTM! Consolidated PR creation simplifies the workflow.The new consolidated approach creates a single PR for version bumps across both platforms, which is cleaner than the previous per-platform approach. The file paths list is comprehensive and covers all version-related files.
1036-1040: Confirm get-version action outputs VERSION
The composite action at.github/actions/get-versionexists and correctly writesVERSIONfrompackage.jsonto$GITHUB_ENV.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/mobile-deploy.yml(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/**/*.{yml,yaml}: In GitHub workflows, use the shared composite actions in .github/actions for dependency caching instead of calling actions/cache directly
Use the cache-yarn composite action for Yarn dependency caching in workflows
Use the cache-bundler composite action for Ruby gems caching in workflows
Use the cache-gradle composite action for Gradle caching in workflows
Use the cache-pods composite action for CocoaPods caching in workflows
Files:
.github/workflows/mobile-deploy.yml
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-deps
- GitHub Check: analyze-ios
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
transphorm
deleted the
justin/improve-auto-pr-logic-allow-for-version-tags
branch
* refine mobile deploy auto pr logic * remove env check * fix get-version
staging: test mobile deploy auto pull request (#1234)
Summary by CodeRabbit
New Features
Chores
Refactor