-
Notifications
You must be signed in to change notification settings - Fork 1
added the build part #39
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
base: master
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a GitHub Actions job to build and upload an Android APK (gated by Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Jobs as Workflow Jobs
participant EAS as EAS CLI
participant Art as Artifact Storage
Dev->>GH: Push / PR triggers workflow
GH->>Jobs: Run lint, format, typecheck
Jobs->>Jobs: Setup Node v20, pnpm v10.12.1, install deps
alt EXPO_TOKEN present
Jobs->>EAS: pnpm dlx eas-cli@latest build --platform android --profile preview --non-interactive --wait --clear-cache
EAS-->>Jobs: Build completes (APK)
Jobs->>EAS: pnpm dlx eas-cli@latest build:download --platform android --latest --profile preview -o ./app.apk
Jobs->>Art: upload-artifact ./app.apk (android-apk)
else EXPO_TOKEN missing
Jobs-->>Jobs: Skip build/download/upload steps
end
sequenceDiagram
autonumber
participant Expo as Expo Prebuild
participant Plugin as with-core-splashscreen-dependency
participant Gradle as android/app/build.gradle
participant Warn as WarningAggregator
Expo->>Plugin: Apply config plugin
Plugin->>Gradle: Read modResults.contents
alt dependency missing
Plugin->>Gradle: Insert implementation("androidx.core:core-splashscreen:1.0.1") and expo aggregator line into dependencies{}
else dependency present
Plugin-->>Gradle: No change
end
Plugin-->>Expo: Return modified modResults
Note right of Plugin: RunOnce ensures single injection
alt error
Plugin->>Warn: addWarningAndroid('core-splashscreen', message)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. Comment |
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.
Pull Request Overview
This PR adds a build job to the existing GitHub Actions workflow for building Android APK files using Expo Application Services (EAS). The build process is designed to run after the existing lint, format, and typecheck jobs complete successfully.
- Adds a new
buildjob that depends on successful completion of code quality checks - Implements Android APK building using EAS CLI with preview profile
- Includes artifact upload functionality for the generated APK
| run: pnpm dlx eas-cli@latest --version | ||
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | ||
| - name: Build Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | ||
| - name: Download Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk |
Copilot
AI
Aug 24, 2025
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.
The EAS CLI version is not pinned, using '@latest' which could lead to unexpected behavior if breaking changes are introduced. Consider pinning to a specific version for reproducible builds.
| run: pnpm dlx eas-cli@latest --version | |
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | |
| - name: Build Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| run: pnpm dlx eas-cli@6.3.0 --version | |
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | |
| - name: Build Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@6.3.0 build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@6.3.0 build:download --platform android --profile preview --latest --path ./app.apk |
| run: pnpm dlx eas-cli@latest --version | ||
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | ||
| - name: Build Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | ||
| - name: Download Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk |
Copilot
AI
Aug 24, 2025
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.
The EAS CLI version is not pinned, using '@latest' which could lead to unexpected behavior if breaking changes are introduced. Consider pinning to a specific version for reproducible builds.
| run: pnpm dlx eas-cli@latest --version | |
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | |
| - name: Build Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| run: pnpm dlx eas-cli@6.3.0 --version | |
| # No explicit login needed, Expo CLI uses EXPO_TOKEN env automatically | |
| - name: Build Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@6.3.0 build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@6.3.0 build:download --platform android --profile preview --latest --path ./app.apk |
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | ||
| - name: Download Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | ||
| # ...existing code... | ||
| - name: Upload Android artifact | ||
| if: env.EXPO_TOKEN != '' |
Copilot
AI
Aug 24, 2025
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.
The condition 'env.EXPO_TOKEN != ''' is repeated three times. Consider using a job-level condition or extracting this to a reusable condition to reduce duplication.
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact |
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | ||
| - name: Download Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | ||
| # ...existing code... | ||
| - name: Upload Android artifact | ||
| if: env.EXPO_TOKEN != '' |
Copilot
AI
Aug 24, 2025
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.
The condition 'env.EXPO_TOKEN != ''' is repeated three times. Consider using a job-level condition or extracting this to a reusable condition to reduce duplication.
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact | |
| if: env.EXPO_TOKEN != '' | |
| if: env.HAS_EXPO_TOKEN == 'true' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.HAS_EXPO_TOKEN == 'true' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact | |
| if: env.HAS_EXPO_TOKEN == 'true' |
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | ||
| - name: Download Android APK | ||
| if: env.EXPO_TOKEN != '' | ||
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | ||
| # ...existing code... | ||
| - name: Upload Android artifact | ||
| if: env.EXPO_TOKEN != '' |
Copilot
AI
Aug 24, 2025
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.
The condition 'env.EXPO_TOKEN != ''' is repeated three times. Consider using a job-level condition or extracting this to a reusable condition to reduce duplication.
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact | |
| if: env.EXPO_TOKEN != '' | |
| run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait | |
| - name: Download Android APK | |
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | |
| # ...existing code... | |
| - name: Upload Android artifact |
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: 2
♻️ Duplicate comments (1)
.github/workflows/expo-build.yml (1)
91-99: Pin EAS CLI for reproducible buildsUsing @latest risks sudden breakages. Pin to a known-good version.
Apply this diff:
- - name: Ensure EAS CLI available - run: pnpm dlx eas-cli@latest --version + - name: Ensure EAS CLI available + run: pnpm dlx eas-cli@6.3.0 --version @@ - - name: Build Android APK + - name: Build Android (capture build id) if: env.EXPO_TOKEN != '' - run: pnpm dlx eas-cli@latest build --platform android --non-interactive --profile preview --clear-cache --wait + id: eas_build + run: | + JSON=$(pnpm dlx eas-cli@6.3.0 build --platform android --non-interactive --profile preview --clear-cache --wait --json) + echo "$JSON" > eas-build.json + BID=$(echo "$JSON" | jq -r '.builds[0].id // .builds[0].buildId') + echo "BUILD_ID=$BID" >> "$GITHUB_OUTPUT" @@ - - name: Download Android APK + - name: Download Android artifact if: env.EXPO_TOKEN != '' - run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk + run: pnpm dlx eas-cli@6.3.0 build:download --platform android --profile preview -i ${{ steps.eas_build.outputs.BUILD_ID }} --path ./app.aab
🧹 Nitpick comments (6)
app.json (1)
54-56: Optional: Guard against accidental inclusion in non-Android contextsNot required, but you could early-return in the plugin when !config.modRequest.platform === 'android' to skip work on iOS/web.
.github/workflows/expo-build.yml (3)
73-90: Speed up jobs with PNPM cacheEnable Node’s pnpm cache to reduce install time across jobs.
Apply this diff in each job’s setup-node step:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 20 + cache: 'pnpm'
73-90: Optional: Add a timeout to the build jobPrevent hung CI runs on long/queued builds.
build: - runs-on: ubuntu-latest + runs-on: ubuntu-latest + timeout-minutes: 90
95-103: Reduce repeated conditionsYou can gate the entire build job or define a reusable env flag to DRY the if: env.EXPO_TOKEN != '' checks.
- build: + build: runs-on: ubuntu-latest needs: [lint, format, typecheck] - env: - EXPO_TOKEN: ${{ secrets.EXPO_TOKEN }} + env: + EXPO_TOKEN: ${{ secrets.EXPO_TOKEN }} + HAS_EXPO_TOKEN: ${{ secrets.EXPO_TOKEN != '' }} @@ - - name: Build Android (capture build id) - if: env.EXPO_TOKEN != '' + - name: Build Android (capture build id) + if: env.HAS_EXPO_TOKEN == 'true' @@ - - name: Download Android artifact - if: env.EXPO_TOKEN != '' + - name: Download Android artifact + if: env.HAS_EXPO_TOKEN == 'true' @@ - - name: Upload Android artifact - if: env.EXPO_TOKEN != '' + - name: Upload Android artifact + if: env.HAS_EXPO_TOKEN == 'true'app.plugin.js (2)
12-23: Avoid shadowing the config identifier for readabilityRename the inner callback param so outer config isn’t shadowed.
-const withCoreSplashscreenDependency = (config) => - withAppBuildGradle(config, (config) => { +const withCoreSplashscreenDependency = (config) => + withAppBuildGradle(config, (mod) => { try { - config.modResults.contents = ensureDependency(config.modResults.contents); + mod.modResults.contents = ensureDependency(mod.modResults.contents); } catch (e) { WarningAggregator.addWarningAndroid( 'core-splashscreen', `Failed to add androidx.core:core-splashscreen dependency: ${e?.message ?? e}` ); } - return config; + return mod; });
7-10: Edge case: missing dependencies blockIf dependencies { } isn’t found (customized build.gradle), the replace no-ops. Consider guarding and warning explicitly.
function ensureDependency(contents) { - if (DEP_REGEX.test(contents)) return contents; - return contents.replace(/dependencies\s*\{/m, (m) => `${m}\n ${DEP_LINE}`); + if (DEP_REGEX.test(contents)) return contents; + if (!/dependencies\s*\{/m.test(contents)) { + WarningAggregator.addWarningAndroid('core-splashscreen', 'No dependencies { } block found in app/build.gradle'); + return contents; + } + return contents.replace(/dependencies\s*\{/m, (m) => `${m}\n ${DEP_LINE}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/expo-build.yml(1 hunks)app.json(1 hunks)app.plugin.js(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: murtazanoori
PR: The-Creative-Programming-Group/finance-io#34
File: .github/workflows/expo-build.yml:10-13
Timestamp: 2025-08-09T18:13:32.225Z
Learning: In the finance-io repository's GitHub Actions workflows, the team prefers to keep the `id-token: write` permission even when not immediately needed, as a future-proofing measure for potential OIDC authentication adoption with Expo EAS builds.
🔇 Additional comments (4)
app.json (1)
54-56: Plugin registration LGTMThe custom plugin is correctly referenced and ordering after expo-splash-screen is fine.
.github/workflows/expo-build.yml (2)
10-13: Permissions match team preferenceKeeping id-token: write aligns with the repo’s stated preference for future OIDC needs.
101-106: No changes needed: eas.json’s preview profile specifiesandroid.buildType: "apk", so uploadingapp.apkasandroid-apkis correct.app.plugin.js (1)
5-5: Retain the current dependency version
androidx.core:core-splashscreen 1.0.1 is already the latest stable release—no 1.1.x stable exists.Likely an incorrect or invalid review comment.
| run: pnpm dlx eas-cli@latest build:download --platform android --profile preview --latest --path ./app.apk | ||
| # ...existing code... | ||
| - name: Upload Android artifact | ||
| if: env.EXPO_TOKEN != '' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: android-apk | ||
| path: ./app.apk |
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.
Avoid --latest; tie download to the build you just triggered
--latest can fetch an unrelated artifact from another commit/branch. The previous diff captures BUILD_ID and downloads that exact build.
🤖 Prompt for AI Agents
.github/workflows/expo-build.yml lines 99-106: the workflow currently uses
`--latest` when running `eas-cli build:download`, which can retrieve an
unrelated artifact; change the command to download the exact build by using the
BUILD_ID captured earlier (e.g., replace `--latest` with `--id ${{ env.BUILD_ID
}}` or the equivalent environment variable you set), keeping the platform,
profile and path flags intact so the download step deterministically fetches the
build you just triggered.
| const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")'; | ||
|
|
||
| function ensureDependency(contents) { | ||
| if (contents.includes(DEP_LINE)) return contents; // already present | ||
| return contents.replace(/dependencies\s*\{/m, (match) => `${match}\n ${DEP_LINE}`); | ||
| } |
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
Make detection robust to avoid duplicate entries
Exact string match can miss alternative Gradle syntaxes or different versions, causing duplicate dependencies. Use a regex that matches both with/without parentheses and any version.
Apply this diff:
-const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")';
+const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")';
+const DEP_REGEX = /implementation\s*\(?["']androidx\.core:core-splashscreen:[^"']+["']\)?/;
@@
-function ensureDependency(contents) {
- if (contents.includes(DEP_LINE)) return contents; // already present
- return contents.replace(/dependencies\s*\{/m, (match) => `${match}\n ${DEP_LINE}`);
-}
+function ensureDependency(contents) {
+ if (DEP_REGEX.test(contents)) return contents;
+ return contents.replace(/dependencies\s*\{/m, (m) => `${m}\n ${DEP_LINE}`);
+}📝 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.
| const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")'; | |
| function ensureDependency(contents) { | |
| if (contents.includes(DEP_LINE)) return contents; // already present | |
| return contents.replace(/dependencies\s*\{/m, (match) => `${match}\n ${DEP_LINE}`); | |
| } | |
| const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")'; | |
| const DEP_REGEX = /implementation\s*\(?["']androidx\.core:core-splashscreen:[^"']+["']\)?/; | |
| function ensureDependency(contents) { | |
| if (DEP_REGEX.test(contents)) return contents; | |
| return contents.replace(/dependencies\s*\{/m, (m) => `${m}\n ${DEP_LINE}`); | |
| } |
…gin resolution on EAS
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: 0
♻️ Duplicate comments (1)
app.plugin.js (1)
9-11: Make dependency detection robust; avoid dupes across Gradle syntaxesUse regex for presence checks so we don’t re-insert if the line exists with single quotes, no parentheses, a different version, or different spacing. This also aligns with the earlier suggestion on this PR thread.
Apply:
const DEP_LINE = 'implementation("androidx.core:core-splashscreen:1.0.1")'; const EXPO_AGG_LINE = 'implementation(project(":expo"))'; +const DEP_REGEX = /implementation\s*\(?["']androidx\.core:core-splashscreen:[^"']+["']\)?/; +const EXPO_AGG_REGEX = /implementation\s*\(?project\s*\(\s*['"]\:expo['"]\s*\)\s*\)?/; ... - if (!updated.includes(EXPO_AGG_LINE)) { + if (!EXPO_AGG_REGEX.test(updated)) { updated = updated.replace( /dependencies\s*\{/m, (match) => `${match}\n ${EXPO_AGG_LINE}`, ); } // Ensure the splashscreen dependency is present - if (!updated.includes(DEP_LINE)) { + if (!DEP_REGEX.test(updated)) { updated = updated.replace( /dependencies\s*\{/m, (match) => `${match}\n ${DEP_LINE}`, ); }Also applies to: 26-33
🧹 Nitpick comments (3)
app.plugin.js (3)
37-57: Skip noisy warnings during introspectionEAS “read config” passes often run; prefer a no-op instead of warning spam when introspecting.
const withCoreSplashscreenDependency = (config) => withAppBuildGradle(config, (config) => { - try { + try { + if (config.modRequest?.introspect) { + return config; // avoid warnings during introspection + } const updated = ensureDependency(config.modResults?.contents)
9-11: Optional: unpin or centralize versioningConsider driving the splashscreen version from a Gradle version catalog (libs.versions.toml) or a root ext to ease upgrades. Keeping 1.0.1 hardcoded is fine short-term.
18-25: Gate expo aggregator insertion
Skip insertingimplementation(project(":expo"))unlessandroid/settings.gradle(.kts)containsinclude(":expo"), for example by wrapping withwithSettingsGradleand checkingsettings.modResults.contentsbefore injecting, or expose anenableExpoAggregatoroption to opt-in.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app.plugin.js(1 hunks)
🔇 Additional comments (3)
app.plugin.js (3)
3-7: LGTM: Correct use of Expo config-plugin APIsImports are correct and minimal for this mod.
12-17: Guarding for missing gradle contents is goodEarly return when gradle contents or dependencies block are absent prevents crashes during introspection.
59-64: Run-once wrapping looks goodStable plugin id and version; export shape is correct.
Add build code etc.
Summary by CodeRabbit
Bug Fixes
Chores