Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Sep 30, 2025

Summary by CodeRabbit

  • Chores

    • Standardized CI/CD environment: added monitoring, crash-reporting, analytics, and Google sign-in keys to iOS and Android build steps for improved observability.
    • Reordered and unified deployment group settings and refined upload condition formatting for clarity.
    • Removed legacy Slack-based deployment notifications and cleaned helper modules accordingly.
  • Impact

    • No change to app functionality; deployment alerts no longer use Slack.
    • More consistent, transparent builds and uploads with enhanced monitoring.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Updated mobile CI: added multiple env vars (Grafana Loki, Segment, Sentry, Google Sign-In, Android keystore alias/password), moved IOS_TESTFLIGHT_GROUPS, removed Slack secrets from Play upload. Removed Slack notification blocks from Fastlane lanes and dropped Slack helper extension.

Changes

Cohort / File(s) Summary
CI workflow env updates
.github/workflows/mobile-deploy.yml
Added env vars: Grafana Loki (URL/USERNAME/PASSWORD), SEGMENT_KEY, SENTRY_DSN, Google Sign-In client ID, ANDROID_KEY_ALIAS, ANDROID_KEY_PASSWORD; ensured IOS_TESTFLIGHT_GROUPS presence and reordered sections; removed Slack secrets from Play upload step and refined conditions/formatting.
Fastlane Slack removal (iOS lanes)
app/fastlane/Fastfile
Deleted Slack notification blocks from iOS lanes (internal_test, deploy_auto) including deploy_source/emoji logic and Slack uploads; core build/upload flow unchanged.
Fastlane Slack removal (Android lanes)
app/fastlane/Fastfile
Deleted Slack notification block from upload_android_build; build/upload logic retained.
Fastlane helper cleanup
app/fastlane/helpers.rb
Removed helpers/slack import and extend Fastlane::Helpers::Slack; retained other helper extensions (Common/IOS/Android/VersionManager).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant GH as GitHub Actions (mobile-deploy.yml)
  participant FL as Fastlane (Fastfile)
  participant ASC as App Store Connect
  participant GP as Google Play

  Dev->>GH: Push tag / trigger workflow
  GH->>FL: Run iOS build (env: SEGMENT_KEY, SENTRY_DSN, GRAFANA_LOKI creds)
  FL-->>FL: Build/archive .ipa
  GH->>ASC: Upload to TestFlight (env: IOS_TESTFLIGHT_GROUPS, SENTRY_DSN)
  Note over GH,FL: Slack notifications removed

  GH->>FL: Run Android build (env: keystore alias/password, SEGMENT_KEY, SENTRY_DSN, GRAFANA_LOKI, Google Sign-In)
  FL-->>FL: Build .aab
  GH->>GP: Upload to Play Store
  Note over GH,GP: Slack tokens/channels removed from upload step
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • remicolin

Poem

Pipelines tidy, secrets arranged,
Slackless lanes now gently changed.
Keys and DSNs take up their posts,
Builds proceed from coast to coast.
Quiet ship — swift sails, ready to range. 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title reads like a branch name with version tags rather than a clear summary of the actual changes, vaguely referencing “fix deploy workflows” without specifying what was fixed or why. While it does touch on the deployment workflows adjusted in this PR, it lacks the concise clarity needed for a teammate to immediately grasp the core modifications. Including “v268 rd2” adds noise rather than value in understanding the change set. Please rename the PR to a clear, single-sentence description of the primary change—e.g. “Remove Slack notifications and update CI deploy workflows environment variables”—so reviewers instantly understand the intent and scope.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50f20f1 and 82cc2fe.

📒 Files selected for processing (1)
  • .github/workflows/mobile-deploy.yml (4 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@transphorm transphorm marked this pull request as ready for review September 30, 2025 21:49
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
.github/workflows/mobile-deploy.yml (1)

759-764: Verify if NDK caching should use a shared composite action.

The NDK cache uses actions/cache@v4 directly, while the coding guidelines state to use shared composite caching actions and avoid calling actions/cache directly. However, there's no cache-ndk action in the listed shared actions.

Is this an intentional exception, or should a shared composite action be created for NDK caching to maintain consistency?

Based on coding guidelines.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3de7ef3 and 50f20f1.

📒 Files selected for processing (3)
  • .github/workflows/mobile-deploy.yml (4 hunks)
  • app/fastlane/Fastfile (0 hunks)
  • app/fastlane/helpers.rb (0 hunks)
💤 Files with no reviewable changes (2)
  • app/fastlane/Fastfile
  • app/fastlane/helpers.rb
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

.github/workflows/**/*.{yml,yaml}: In GitHub Actions workflows, use shared composite caching actions from .github/actions (cache-yarn, cache-bundler, cache-gradle, cache-pods)
Do not call actions/cache directly; rely on the shared composite caching actions
When using cache actions, optionally pass cache-version (often with GH_CACHE_VERSION and tool version) for stable keys

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). (5)
  • GitHub Check: e2e-ios
  • GitHub Check: android-build-test
  • GitHub Check: build-deps
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (3)
.github/workflows/mobile-deploy.yml (3)

484-486: LGTM! Observability and authentication credentials properly configured.

The addition of Grafana Loki, Segment, Sentry, and TestFlight group environment variables for the iOS build follows secure practices by referencing secrets rather than hardcoding values.

Also applies to: 501-501, 503-503, 505-505


892-893: LGTM! Android keystore and observability credentials properly added.

The keystore signing credentials and observability stack environment variables are correctly configured, mirroring the iOS build setup. The earlier verification step already validates these credentials.

Also applies to: 898-901, 903-904


925-925: Good fix! Boolean comparison is now type-safe.

The condition now correctly compares inputs.test_mode (a boolean) to true rather than the string 'true', which is more accurate given the workflow input type definition.

@transphorm transphorm merged commit 50f670d into dev Sep 30, 2025
15 of 16 checks passed
@transphorm transphorm deleted the justin/fix-deploy-workflows-v268-rd2 branch September 30, 2025 21:56
transphorm added a commit that referenced this pull request Sep 30, 2025
chore: fix mobile deploy pipelines v2.6.8 rd2 (#1159)
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.

2 participants