fix(fastlane): incorporated iOS related changes from template - #2639
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds iOS primary locale support (en-GB), enhances Fastlane lanes for sanitization, CocoaPods, version/TestFlight and locale-aware metadata, introduces CI/CD workflows and documentation, updates Gemfile and CI Gradle settings, and overhauls secrets management with dynamic env-file resolution replacing committed Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🧹 Nitpick comments (1)
fastlane/FastFile (1)
573-595: Extract duplicated version reconciliation logic into a shared helper.The same “compare with latest TestFlight version and bump patch” block is duplicated in beta/release. This is a drift risk for future fixes.
Refactor sketch
+ private_lane :resolve_appstore_version do |options| + requested = options[:requested_version] + latest = options[:latest_testflight_version] + version = requested + if latest && !latest.to_s.empty? + begin + if Gem::Version.new(version) <= Gem::Version.new(latest) + parts = latest.split('.') + parts[-1] = (parts[-1].to_i + 1).to_s + version = parts.join('.') + end + rescue ArgumentError => e + UI.error("Version comparison failed: #{e.message}, using #{version}") + end + end + version + endAlso applies to: 699-720
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fastlane/FastFile` around lines 573 - 595, Extract the duplicated "compare with latest TestFlight version and bump patch" logic into a single helper method (e.g., reconcile_version_with_testflight) that accepts the candidate version and latest_version and returns the final version; move the Gem::Version comparison, parts[-1] patch increment, UI.important bump message and the rescue ArgumentError UI.error handling into that helper, then replace both inline blocks in the beta and release lanes with a call to reconcile_version_with_testflight(version, latest_version) before calling increment_version_number (xcodeproj: ios_config[:project_path], version_number: version); ensure the helper preserves the same behavior and messages and returns the original candidate when comparison or parsing fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fastlane/FastFile`:
- Around line 809-810: Before calling deliver in FastFile, add a pre-check that
verifies ios_config[:app_rating_config_path] exists and is a readable file and
fail fast with a clear error if not; locate the deliver invocation and validate
the value of ios_config[:app_rating_config_path] (and/or add this check into
validate_config) and raise a user-friendly error (e.g., via UI.user_error or
abort) when the path is nil, missing, or not a file so deliver is only called
with a valid age-rating config path.
---
Nitpick comments:
In `@fastlane/FastFile`:
- Around line 573-595: Extract the duplicated "compare with latest TestFlight
version and bump patch" logic into a single helper method (e.g.,
reconcile_version_with_testflight) that accepts the candidate version and
latest_version and returns the final version; move the Gem::Version comparison,
parts[-1] patch increment, UI.important bump message and the rescue
ArgumentError UI.error handling into that helper, then replace both inline
blocks in the beta and release lanes with a call to
reconcile_version_with_testflight(version, latest_version) before calling
increment_version_number (xcodeproj: ios_config[:project_path], version_number:
version); ensure the helper preserves the same behavior and messages and returns
the original candidate when comparison or parsing fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0adff8be-2324-42a4-8c45-0a2702700584
📒 Files selected for processing (4)
fastlane-config/ios_config.rbfastlane-config/project_config.rbfastlane/FastFilefastlane/age_rating.json
| # Age rating declarations (lootBox, parentalControls, advertising, etc.) | ||
| app_rating_config_path: ios_config[:app_rating_config_path] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify age-rating file existence and validation logic
echo "=== Searching for age_rating files ==="
fd -a 'age_rating.json' 2>/dev/null || echo "No age_rating.json found"
echo ""
echo "=== Searching for validation and configuration references ==="
rg -n -C3 'validate_config|required_files|age_rating_config_path|app_rating_config_path' fastlane-config/project_config.rb fastlane/FastFile 2>/dev/null || echo "Search may have failed - files might not exist"
echo ""
echo "=== Checking if fastlane/FastFile exists and examining lines 809-815 ==="
if [ -f fastlane/FastFile ]; then
echo "File exists. Reading context around lines 809-815:"
sed -n '800,820p' fastlane/FastFile
else
echo "fastlane/FastFile not found"
fi
echo ""
echo "=== Checking project_config validation ==="
if [ -f fastlane-config/project_config.rb ]; then
echo "File exists. Searching for validation methods:"
rg -n 'def.*valid|def.*required|def.*check' fastlane-config/project_config.rb
else
echo "fastlane-config/project_config.rb not found"
fiRepository: openMF/mifos-x-field-officer-app
Length of output: 3426
🏁 Script executed:
#!/bin/bash
# Check for any validation between validate_config call and deliver call
echo "=== Looking for validate_config call and subsequent validation ==="
rg -n -B5 -A15 'validate_config' fastlane/FastFile
echo ""
echo "=== Checking full validate_config implementation ==="
sed -n '262,290p' fastlane-config/project_config.rbRepository: openMF/mifos-x-field-officer-app
Length of output: 1334
🏁 Script executed:
#!/bin/bash
# Search for where validate_config is called and look for any other validations
echo "=== Searching for validate_config call in FastFile ==="
rg -n 'validate_config' fastlane/FastFile
echo ""
echo "=== Looking at the deploy lane structure around line 810 ==="
sed -n '750,815p' fastlane/FastFile
echo ""
echo "=== Check if app_rating_config_path is used anywhere else ==="
rg -n 'app_rating_config_path' fastlane/
</parameter>Repository: openMF/mifos-x-field-officer-app
Length of output: 3650
Add pre-check for age-rating config file before deliver call.
Line 810 passes ios_config[:app_rating_config_path] to deliver() without validation. The validate_config method does not check this file, so a missing or invalid path will cause a downstream Fastlane error instead of failing fast with a clear message.
Suggested guard
+ unless File.exist?(ios_config[:app_rating_config_path])
+ UI.user_error!("Missing age rating config: #{ios_config[:app_rating_config_path]}")
+ end
+
deliver(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fastlane/FastFile` around lines 809 - 810, Before calling deliver in
FastFile, add a pre-check that verifies ios_config[:app_rating_config_path]
exists and is a readable file and fail fast with a clear error if not; locate
the deliver invocation and validate the value of
ios_config[:app_rating_config_path] (and/or add this check into validate_config)
and raise a user-friendly error (e.g., via UI.user_error or abort) when the path
is nil, missing, or not a file so deliver is only called with a valid age-rating
config path.
|
@techsavvy185 Can you please tell us the steps taken and references followed to implement this PR? |
|
@biplab1 I took this PR from the KMP template for reference and basically changed the files accordingly. |
|
@techsavvy185 You had to refer a PR from the mifos-pay (mobile-wallet) as mentioned in the ticket. Please check. |
|
@biplab1 I have made all the changes according to that PR. I, however, am not sure about the keystore files. Could you please guide me on that one? |
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
Gemfile (1)
6-12: Optional: keep Gemfile gems alphabetically ordered.This avoids Bundler/OrderedGems warnings and keeps diffs cleaner.
♻️ Proposed ordering
gem "abbrev" gem "base64" gem "mutex_m" gem "bigdecimal" -gem "fastlane" gem "cocoapods" +gem "fastlane"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gemfile` around lines 6 - 12, The Gemfile gem list is not alphabetically ordered; reorder the gem entries (e.g., "abbrev", "base64", "bigdecimal", "fastlane", "mutex_m", "cocoapods") so they are sorted alphabetically to satisfy Bundler/OrderedGems and keep diffs clean—update the Gemfile to place each gem line in lexicographic order and save the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/ci-gradle.properties:
- Around line 1-17: The CI-only Gradle properties in
.github/ci-gradle.properties (e.g., org.gradle.daemon, org.gradle.parallel,
org.gradle.jvmargs, org.gradle.workers.max, kotlin.incremental,
kotlin.compiler.execution.strategy) are never applied because workflows don't
load that file; update the GitHub workflow(s) that call the reusable external
workflow to explicitly pass these settings to Gradle—either export them as
environment variables (e.g., GRADLE_OPTS or ORG_GRADLE_PROJECT_* for each
property) or append -D flags to the Gradle command (e.g.,
-Dorg.gradle.daemon=false -Dkotlin.incremental=false etc.); alternatively,
modify the reusable workflow input interface so callers can submit CI Gradle
settings and ensure the workflow applies those to the Gradle invocation.
In @.github/CLAUDE.md:
- Around line 553-560: The table's numeric counts don't match the listed
secrets: e.g. the "Firebase" row shows count `3` but lists only `FIREBASECREDS`
and `GOOGLESERVICES`, and the `Total: 30+ secrets` doesn't match the sum of
category counts; update the table so each category count (Firebase, Play Store,
iOS, Desktop, Shared) exactly matches the number of secret names shown (or add
any missing secret names), and then recalculate and replace the "Total" value
with the correct exact sum (remove "30+" unless that is accurate), ensuring
consistency between the numeric counts and the listed secret identifiers like
FIREBASECREDS, GOOGLESERVICES, PLAYSTORECREDS, APPSTORE_KEY_ID,
MACOS_SIGNING_CERTIFICATE, GITHUB_TOKEN, etc.
- Line 570: Update the docs to match the filename expected by the keystore
manager: change the table entry that currently lists `Auth_key.p8` to
`AuthKey.p8` so it aligns with the filename used in `keystore-manager.sh` and
ensures `APPSTORE_AUTH_KEY` is encoded correctly; specifically, update the
string in the CLAUDE.md table to `AuthKey.p8` wherever `Auth_key.p8` appears and
verify consistency with the `keystore-manager.sh` mapping for
`APPSTORE_AUTH_KEY`.
- Around line 26-34: The markdown code fence showing the architecture diagram
uses an unlabeled triple-backtick block; change the opening fence from ``` to
```text so the block is language-tagged (e.g., replace the opening ``` before
"Local Workflows (.github/workflows/)" with ```text) while leaving the closing
``` unchanged to satisfy markdownlint MD040.
In @.github/workflows/build-and-deploy-site.yml:
- Around line 2-3: The workflow header comment claims deployments on merges to
dev but the workflow only has workflow_dispatch; either update the comment to
indicate manual trigger only or add a push trigger for the dev branch so they
match—specifically edit the file's top comment and the YAML triggers: either
change the on: section to include push: branches: [dev] alongside
workflow_dispatch (or replace workflow_dispatch) or reword the top comment to
state that the workflow is manual (workflow_dispatch) only; ensure you reference
the workflow_dispatch trigger and the push branches: dev configuration when
making the change.
In @.github/workflows/multi-platform-build-and-publish.yml:
- Line 113: The workflow is enabling CocoaPods with the use_cocoapods: true flag
but the repo lacks a Podfile or Kotlin CocoaPods plugin; remove or set
use_cocoapods to false in the workflow (or guard it behind a conditional) so the
iOS release job doesn't try to run CocoaPods, and if CocoaPods is actually
required add a Podfile or enable the Kotlin CocoaPods plugin in the project;
look for the use_cocoapods key in the workflow and update it accordingly.
In @.github/workflows/pr-check.yml:
- Line 92: The workflow enables CocoaPods (use_cocoapods: true) while the repo
has no Podfile and no CocoaPods plugin configured, which will break the iOS/KMP
build when build_ios: true is set; change use_cocoapods to false in the PR check
workflow to disable CocoaPods, or if CocoaPods is required add a Podfile and the
cocoapods Gradle plugin (the plugin named "cocoapods" and any Podfile entries)
to the Kotlin Multiplatform build so the iOS step and build_ios: true are
consistent.
In `@fastlane/CLAUDE.md`:
- Around line 33-41: Multiple fenced code blocks in CLAUDE.md are missing
language identifiers and trigger MD040; update each triple-backtick block (the
one containing "GitHub Actions ↓ ... App Store", the block with
"-Pandroid.injected.signing.*" properties, and the "fastlane-config/" tree
block, plus the other occurrences at lines 86-91 and 504-517) by adding a
language token such as "text" immediately after the opening ``` so they become
```text to silence MD040 and preserve formatting.
- Line 4: The header "Total Lanes: 12 (7 Android + 5 iOS)" is inconsistent with
the document body which lists 8 Android lanes and 5 iOS lanes; update the header
to reflect the correct totals (change it to "Total Lanes: 13 (8 Android + 5
iOS)") or adjust the body so counts match—edit the header text shown in the line
containing "Total Lanes:" to match the actual lane counts documented.
In `@fastlane/metadata/en-US/release_notes.txt`:
- Line 1: The release notes were updated in fastlane/metadata/en-US but the
project primary locale is set to en-GB and the iOS release lane (FastFile
release lane) writes to the primary locale path built from the project_config
primary locale; move the changes from fastlane/metadata/en-US/release_notes.txt
into fastlane/metadata/en-GB/release_notes.txt (or change the primary locale in
project_config.rb to en-US if that is the intended target) so the FastFile
release lane picks up the updated notes.
- Line 2: Remove the Git trailer line starting with "Co-authored-by:" from the
release notes so only user-facing content remains; open release_notes.txt,
delete the line that begins with "Co-authored-by:" (and any other commit-trailer
lines such as "Signed-off-by:" or similar if present), and ensure the final file
contains only the intended App Store release note text.
In `@keystore-manager.sh`:
- Around line 585-586: Update the stale "Looking for:" hint string in
keystore-manager.sh so it lists the newly supported macOS artifacts; locate the
echo that prints "${YELLOW}Looking for:
firebaseAppDistributionServiceCredentialsFile.json, google-services.json,
playStorePublishServiceCredentialsFile.json, AuthKey.p8, match_ci_key${NC}" and
expand it to include the mac_* cert/profile filenames (e.g., mac_cert*,
mac_profile*, mac_* keys) so the message accurately reflects all supported
files.
- Around line 98-105: The prompt inside resolve_env_file() that reads user input
when both secrets_dir_exists and root_exists are true can hang in
non-interactive environments; before calling read (the case handling for
"choice"), add a non-interactive guard: check if stdin is a TTY (e.g. [ -t 0 ])
or honor a NONINTERACTIVE/CI env var and if false/non-interactive, skip the
prompt and choose the default (option 1) or emit a clear message and return an
error code; otherwise proceed to the existing read prompt and case branch.
Ensure you reference resolve_env_file(), the read -r -p prompt and the case
"$choice" block so the change is applied exactly around that prompt.
---
Nitpick comments:
In `@Gemfile`:
- Around line 6-12: The Gemfile gem list is not alphabetically ordered; reorder
the gem entries (e.g., "abbrev", "base64", "bigdecimal", "fastlane", "mutex_m",
"cocoapods") so they are sorted alphabetically to satisfy Bundler/OrderedGems
and keep diffs clean—update the Gemfile to place each gem line in lexicographic
order and save the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a1735bd-84ce-4db7-b99c-c252a9006adb
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/CLAUDE.md.github/ci-gradle.properties.github/workflows/build-and-deploy-site.yml.github/workflows/cache-cleanup.yaml.github/workflows/monthly-release.yaml.github/workflows/multi-platform-build-and-publish.yml.github/workflows/pr-check.yml.github/workflows/promote-to-production.ymlGemfilefastlane/CLAUDE.mdfastlane/age_rating.jsonfastlane/metadata/en-US/release_notes.txtkeystore-manager.shsecrets.envsecrets.env.template
💤 Files with no reviewable changes (1)
- secrets.env
🚧 Files skipped from review as they are similar to previous changes (1)
- fastlane/age_rating.json
| ``` | ||
| Local Workflows (.github/workflows/) | ||
| ↓ | ||
| Reusable Workflows (mifos-x-actionhub/.github/workflows/) | ||
| ↓ | ||
| Custom Actions (mifos-x-actionhub-*/) | ||
| ↓ | ||
| Fastlane Lanes (fastlane/Fastfile) | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the architecture code fence.
This currently triggers markdownlint MD040.
🧹 Proposed fix
-```
+```text
Local Workflows (.github/workflows/)
↓
Reusable Workflows (mifos-x-actionhub/.github/workflows/)
↓
Custom Actions (mifos-x-actionhub-*/)
↓
Fastlane Lanes (fastlane/Fastfile)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/CLAUDE.md around lines 26 - 34, The markdown code fence showing the
architecture diagram uses an unlabeled triple-backtick block; change the opening
fence from ``` to ```text so the block is language-tagged (e.g., replace the
opening ``` before "Local Workflows (.github/workflows/)" with ```text) while
leaving the closing ``` unchanged to satisfy markdownlint MD040.
| # It ensures that whenever changes are merged into the dev branch or when manually triggered, the web application is built, | ||
| # packaged, and deployed to the GitHub Pages environment, making it accessible online. |
There was a problem hiding this comment.
Workflow description and trigger behavior are inconsistent.
Line [2]-Line [3] claims deployment on merges to dev, but Line [45]-Line [46] configures manual trigger only (workflow_dispatch). Please align either comments or triggers.
Documentation-only correction example
-# It ensures that whenever changes are merged into the dev branch or when manually triggered, the web application is built,
-# packaged, and deployed to the GitHub Pages environment, making it accessible online.
+# It can be manually triggered to build, package, and deploy the web application
+# to the GitHub Pages environment.Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-and-deploy-site.yml around lines 2 - 3, The workflow
header comment claims deployments on merges to dev but the workflow only has
workflow_dispatch; either update the comment to indicate manual trigger only or
add a push trigger for the dev branch so they match—specifically edit the file's
top comment and the YAML triggers: either change the on: section to include
push: branches: [dev] alongside workflow_dispatch (or replace workflow_dispatch)
or reword the top comment to state that the workflow is manual
(workflow_dispatch) only; ensure you reference the workflow_dispatch trigger and
the push branches: dev configuration when making the change.
| ``` | ||
| GitHub Actions | ||
| ↓ | ||
| Custom Actions (inflate secrets, call Fastlane) | ||
| ↓ | ||
| Fastlane Lanes (build, sign, upload) | ||
| ↓ | ||
| Firebase / Play Store / TestFlight / App Store | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
These blocks currently trigger MD040 warnings.
🧹 Proposed fix
-```
+```text
GitHub Actions
↓
Custom Actions (inflate secrets, call Fastlane)
↓
Fastlane Lanes (build, sign, upload)
↓
Firebase / Play Store / TestFlight / App Store-
-Pandroid.injected.signing.store.file=keystores/release_keystore.keystore -Pandroid.injected.signing.store.password=xxx -Pandroid.injected.signing.key.alias=xxx -Pandroid.injected.signing.key.password=xxx
- +text
fastlane-config/
├── project_config.rb # Master configuration (ANDROID, IOS, IOS_SHARED)
├── android_config.rb # Android-specific helpers
└── ios_config.rb # iOS-specific helpers
...
Also applies to: 86-91, 504-517
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fastlane/CLAUDE.md` around lines 33 - 41, Multiple fenced code blocks in
CLAUDE.md are missing language identifiers and trigger MD040; update each
triple-backtick block (the one containing "GitHub Actions ↓ ... App Store", the
block with "-Pandroid.injected.signing.*" properties, and the "fastlane-config/"
tree block, plus the other occurrences at lines 86-91 and 504-517) by adding a
language token such as "text" immediately after the opening ``` so they become
```text to silence MD040 and preserve formatting.
| @@ -0,0 +1,2 @@ | |||
| fix(fastlane): Incorporated iOS related changes from template. (#2639) | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify configured iOS primary locale and release-notes path resolution.
rg -n "primary_locale|localized_build_info|metadata_path|release_notes" fastlane-config fastlane/FastFile
fd release_notes.txt fastlane/metadata -t fRepository: openMF/mifos-x-field-officer-app
Length of output: 1943
🏁 Script executed:
#!/bin/bash
# Check if en-GB release_notes.txt exists and list all locale directories
ls -la fastlane/metadata/
fd release_notes.txt fastlane/metadataRepository: openMF/mifos-x-field-officer-app
Length of output: 336
Update release notes in the correct locale directory.
The primary locale is configured as en-GB (fastlane-config/project_config.rb:77), but this PR updates fastlane/metadata/en-US/release_notes.txt. The iOS release lane (FastFile:750) constructs the release notes path using the configured primary locale and will write to fastlane/metadata/en-GB/release_notes.txt. Updates to the en-US directory will be ignored during the iOS release process. Move the changes to fastlane/metadata/en-GB/release_notes.txt or update the primary locale if en-US is the intended target.
🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Use a hyphen to join words.
Context: fix(fastlane): Incorporated iOS related changes from template. (#2639) C...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fastlane/metadata/en-US/release_notes.txt` at line 1, The release notes were
updated in fastlane/metadata/en-US but the project primary locale is set to
en-GB and the iOS release lane (FastFile release lane) writes to the primary
locale path built from the project_config primary locale; move the changes from
fastlane/metadata/en-US/release_notes.txt into
fastlane/metadata/en-GB/release_notes.txt (or change the primary locale in
project_config.rb to en-US if that is the intended target) so the FastFile
release lane picks up the updated notes.
| @@ -0,0 +1,2 @@ | |||
| fix(fastlane): Incorporated iOS related changes from template. (#2639) | |||
| Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> No newline at end of file | |||
There was a problem hiding this comment.
Remove Git trailer from user-facing release notes.
Line 2 (Co-authored-by: ...) is commit metadata and should not be published as App Store release notes.
✂️ Proposed fix
fix(fastlane): Incorporated iOS related changes from template. (`#2639`)
-Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fastlane/metadata/en-US/release_notes.txt` at line 2, Remove the Git trailer
line starting with "Co-authored-by:" from the release notes so only user-facing
content remains; open release_notes.txt, delete the line that begins with
"Co-authored-by:" (and any other commit-trailer lines such as "Signed-off-by:"
or similar if present), and ensure the final file contains only the intended App
Store release note text.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/multi-platform-build-and-publish.yml (1)
113-113:⚠️ Potential issue | 🟠 MajorDisable CocoaPods until the repo is actually wired for it.
Still seeing the same blocker here. The reusable workflow only expects
use_cocoapods: truewhen the shared module is integrated into the iOS app through CocoaPods /pod install; if that wiring is still absent in this repo, keeping Line 113 enabled will keep the iOS release path on a setup it cannot satisfy. (github.com)#!/bin/bash set -euo pipefail echo "=== Podfiles ===" fd -HI '^Podfile$' . echo echo "=== Kotlin CocoaPods configuration ===" rg -n 'kotlin\("native\.cocoapods"\)|\bcocoapods\s*\{' -g '!**/Pods/**' .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/multi-platform-build-and-publish.yml at line 113, The workflow sets use_cocoapods: true which enables CocoaPods in the reusable job but this repo isn’t wired for CocoaPods; change the use_cocoapods value in the multi-platform-build-and-publish.yml workflow (the use_cocoapods: true setting) to false or remove that key so the reusable workflow won’t attempt a pod install; update the single occurrence of use_cocoapods in the file and re-run the provided Podfile/CocoaPods detection commands to verify no Podfile/CocoaPods config is expected.
🧹 Nitpick comments (2)
.github/workflows/multi-platform-build-and-publish.yml (2)
104-104: Pin the reusable workflow to an immutable SHA.This job hands an external reusable workflow signing keys, store credentials, and
GITHUB_TOKEN. GitHub’s security guidance recommends pinning reusable workflows to a full-length commit SHA, because tags likev1.0.11are mutable. (docs.github.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/multi-platform-build-and-publish.yml at line 104, The reusable workflow invocation using "uses: openMF/mifos-x-actionhub/.github/workflows/multi-platform-build-and-publish.yaml@v1.0.11" must be pinned to an immutable commit SHA; update that uses reference to the full 40-character commit SHA for the external repo (replace "@v1.0.11" with "@<full-commit-sha>"). Locate the uses line in the workflow (the string starting with openMF/mifos-x-actionhub/.github/workflows/multi-platform-build-and-publish.yaml) and fetch the corresponding commit SHA from the external repository (via git or the GitHub UI) before replacing the ref so the workflow points to the exact commit.
17-52: The setup header is already out of sync with the actual workflow.The "REQUIRED SECRETS" block stops at notarization even though this job later forwards App Store Connect, Match, desktop signing, keychain/provisioning, Supabase, and token secrets. The "WORKFLOW INPUTS" block also mentions flags like
build_iosandpublish_iosthat are not exposed byworkflow_dispatch. I’d either update this header or trim it down to a single source of truth so future edits don’t keep drifting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/multi-platform-build-and-publish.yml around lines 17 - 52, The header comments ("REQUIRED SECRETS" and "WORKFLOW INPUTS") are out of sync with the actual workflow_dispatch and job steps; update them to match the real secrets and inputs used (add the App Store Connect/FASTLANE/MATCH/desktop signing/keychain/provisioning/Supabase/token secrets that the job forwards and remove mentions of inputs like build_ios/publish_ios that are not declared in workflow_dispatch), or replace the long lists with a single line pointing to the canonical source (the workflow_dispatch inputs and job/step envs) to avoid drift; ensure the header names "REQUIRED SECRETS" and "WORKFLOW INPUTS" and any example variables in the comment match the actual variables used later in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@keystore-manager.sh`:
- Around line 605-606: update_secrets_env_with_files() currently no-ops if
$ENV_FILE is missing which lets encode_secrets_directory_files() encode file
secrets without persisting them; change the flow so $ENV_FILE is
created/populated before any encoding: add an ensure_env_file_exists() step (or
have update_secrets_env_with_files() bootstrap $ENV_FILE from
secrets.env.template when absent) and call it before
encode_secrets_directory_files() and before the logic around the current
file-creation block (the code around the 684-688/2028-2036 areas) so file
secrets are saved on first run.
- Around line 68-69: The script uses Bash 4+ features (declare -A
IOS_STRING_SECRETS and declare -g) which break on macOS stock Bash 3; add a
startup guard that checks BASH_VERSINFO (or $BASH_VERSION) and if the major
version is less than 4 (or less than 4.2 if you need declare -g semantics)
either exit with a clear error asking the user to run the script with a newer
Bash, or implement a fallback path that avoids associative arrays and global
declare usage (e.g., use plain variables, prefixed names, or temporary files)
for the places referencing IOS_STRING_SECRETS and the locations using declare
-g; update all spots noted (symbols: IOS_STRING_SECRETS, any uses of declare -g)
to follow the chosen approach.
- Around line 489-491: The sed replacement breaks when values contain double
quotes or the chosen delimiter; create two helpers—escape_for_env(value) that
returns a value with backslashes and double quotes escaped for dotenv (so it can
be wrapped safely in double quotes), and escape_for_sed(value) that escapes
backslash, ampersand and the sed delimiter (|) for use in the replacement
portion. Replace the current escaped_value assignment with:
tmp_env=$(escape_for_env "$value"); escaped_value=$(escape_for_sed "$tmp_env");
then use sed as before: sed -i.bak "s|^${key}=.*|${key}=\"${escaped_value}\"|"
"$SECRETS_FILE". Apply the same helper usage to the other sed invocations that
build replacements (the occurrences that currently perform sed -i.bak
"s|^...|...|") so all env-file quoting and sed-delimiter cases are handled.
---
Duplicate comments:
In @.github/workflows/multi-platform-build-and-publish.yml:
- Line 113: The workflow sets use_cocoapods: true which enables CocoaPods in the
reusable job but this repo isn’t wired for CocoaPods; change the use_cocoapods
value in the multi-platform-build-and-publish.yml workflow (the use_cocoapods:
true setting) to false or remove that key so the reusable workflow won’t attempt
a pod install; update the single occurrence of use_cocoapods in the file and
re-run the provided Podfile/CocoaPods detection commands to verify no
Podfile/CocoaPods config is expected.
---
Nitpick comments:
In @.github/workflows/multi-platform-build-and-publish.yml:
- Line 104: The reusable workflow invocation using "uses:
openMF/mifos-x-actionhub/.github/workflows/multi-platform-build-and-publish.yaml@v1.0.11"
must be pinned to an immutable commit SHA; update that uses reference to the
full 40-character commit SHA for the external repo (replace "@v1.0.11" with
"@<full-commit-sha>"). Locate the uses line in the workflow (the string starting
with
openMF/mifos-x-actionhub/.github/workflows/multi-platform-build-and-publish.yaml)
and fetch the corresponding commit SHA from the external repository (via git or
the GitHub UI) before replacing the ref so the workflow points to the exact
commit.
- Around line 17-52: The header comments ("REQUIRED SECRETS" and "WORKFLOW
INPUTS") are out of sync with the actual workflow_dispatch and job steps; update
them to match the real secrets and inputs used (add the App Store
Connect/FASTLANE/MATCH/desktop signing/keychain/provisioning/Supabase/token
secrets that the job forwards and remove mentions of inputs like
build_ios/publish_ios that are not declared in workflow_dispatch), or replace
the long lists with a single line pointing to the canonical source (the
workflow_dispatch inputs and job/step envs) to avoid drift; ensure the header
names "REQUIRED SECRETS" and "WORKFLOW INPUTS" and any example variables in the
comment match the actual variables used later in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 464af1d4-3e88-4663-8443-0b017bf2ba8c
📒 Files selected for processing (4)
.github/workflows/monthly-version-tag.yaml.github/workflows/multi-platform-build-and-publish.ymlkeystore-manager.shkeystores/original_keystore.keystore
| # Global associative array for iOS string secrets | ||
| declare -A IOS_STRING_SECRETS |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does the default /bin/bash shipped with macOS support associative arrays (declare -A) and declare -g? Which Bash version is bundled by default?
💡 Result:
No.
- Associative arrays (
declare -A): Not supported by the/bin/bashthat ships with macOS, because macOS still bundles Bash 3.2.x, anddeclare -A(associative arrays) was introduced in Bash 4. [1] - Global declarations (
declare -g): Not supported in macOS’s default/bin/basheither, becausedeclare -gwas added in Bash 4.2. [2]
Which Bash version is bundled by default?
macOS ships GNU Bash 3.2.57(1)-release as /bin/bash (commonly cited as the bundled version, including on modern macOS releases for compatibility/licensing reasons). [3]
Sources
[1] Bash Hackers Wiki (arrays; notes declare -A introduced with Bash 4) (bash-hackers.gabe565.com)
[2] Greg’s Wiki / Wooledge BashFAQ (notes Bash 4.2 adds declare -g) (mywiki.wooledge.org)
[3] The Register (macOS ships bash 3.2.57(1)) (theregister.com)
Citations:
- 1: https://bash-hackers.gabe565.com/syntax/arrays/?utm_source=openai
- 2: https://mywiki.wooledge.org/BashFAQ/006?utm_source=openai
- 3: https://www.theregister.com/2019/06/04/apple_zsh_macos_catalina_default/?utm_source=openai
Script fails on stock macOS: declare -A and declare -g require Bash 4+, but /bin/bash on macOS is Bash 3.2.x.
The default /bin/bash on macOS is version 3.2.57(1), which predates Bash 4 and does not support associative arrays (declare -A, introduced in Bash 4) or global declarations (declare -g, added in Bash 4.2). The script will abort on any macOS system using the stock shell unless a newer Bash is installed and explicitly used.
Applies to lines 68–69, 425, 540, 617, and 967.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@keystore-manager.sh` around lines 68 - 69, The script uses Bash 4+ features
(declare -A IOS_STRING_SECRETS and declare -g) which break on macOS stock Bash
3; add a startup guard that checks BASH_VERSINFO (or $BASH_VERSION) and if the
major version is less than 4 (or less than 4.2 if you need declare -g semantics)
either exit with a clear error asking the user to run the script with a newer
Bash, or implement a fallback path that avoids associative arrays and global
declare usage (e.g., use plain variables, prefixed names, or temporary files)
for the places referencing IOS_STRING_SECRETS and the locations using declare
-g; update all spots noted (symbols: IOS_STRING_SECRETS, any uses of declare -g)
to follow the chosen approach.
|
@techsavvy185 Please resolve the merge conflicts. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/pr-check.yml (1)
72-72: Fix the stale trigger comment.Line 72 now filters
pull_requesttargets for bothdevanddevelopment, but the inline comment still says "pushes to dev branch". Updating or dropping it will keep the workflow docs aligned with the actual trigger.Proposed fix
- branches: [ dev, development ] # Runs on pushes to dev branch + branches: [ dev, development ] # Runs for PRs targeting dev or development🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-check.yml at line 72, Update the stale inline comment next to the YAML key branches: [ dev, development ] so it accurately describes the trigger (this workflow filters pull_request targets for both dev and development) or remove the comment entirely; locate the comment adjacent to the branches entry in pr-check.yml and replace "Runs on pushes to dev branch" with a concise phrase like "Runs on pull requests targeting dev and development" (or simply delete the comment).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/pr-check.yml:
- Line 72: Update the stale inline comment next to the YAML key branches: [ dev,
development ] so it accurately describes the trigger (this workflow filters
pull_request targets for both dev and development) or remove the comment
entirely; locate the comment adjacent to the branches entry in pr-check.yml and
replace "Runs on pushes to dev branch" with a concise phrase like "Runs on pull
requests targeting dev and development" (or simply delete the comment).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb942d3f-91f5-44d8-9743-5535c3560a9f
📒 Files selected for processing (1)
.github/workflows/pr-check.yml
biplab1
changed the title
Co-authored-by: Biplab Dutta <biplabdutta27@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/multi-platform-build-and-publish.yml (1)
113-113:⚠️ Potential issue | 🟠 MajorDisable CocoaPods for now to avoid iOS lane failure.
Line 113 still sets
use_cocoapods: true, but this has already been flagged and requested to be set false. Keep it disabled until Podfile/CocoaPods config is actually present.Proposed fix
- use_cocoapods: true + use_cocoapods: false#!/bin/bash set -euo pipefail echo "=== Check workflow setting ===" rg -nP '^\s*use_cocoapods:\s*' .github/workflows/multi-platform-build-and-publish.yml echo "=== Check Podfile presence ===" fd -HI '^Podfile$' || true echo "=== Check Kotlin CocoaPods DSL/plugin usage ===" rg -nP 'kotlin\("native\.cocoapods"\)|^\s*cocoapods\s*\{' -g '*.kts' -g '*.gradle' || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/multi-platform-build-and-publish.yml at line 113, Update the GitHub Actions workflow to disable CocoaPods by changing the use_cocoapods setting from true to false; locate the use_cocoapods entry in the multi-platform-build-and-publish workflow (look for the symbol use_cocoapods) and set it to false so iOS lanes don’t attempt Pod operations until Podfile/CocoaPods config is added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/multi-platform-build-and-publish.yml:
- Around line 37-47: The workflow comment block in
.github/workflows/multi-platform-build-and-publish.yml is stale: it lists inputs
publish_android, build_ios, and publish_ios that are not defined under
workflow_dispatch; update the documentation to match the actual
workflow_dispatch inputs (remove the three obsolete entries or add the
corresponding workflow_dispatch inputs if they are intended features). Locate
the block containing the commented "WORKFLOW INPUTS" and the referenced symbols
publish_android, build_ios, publish_ios and either delete those lines or add the
missing inputs under workflow_dispatch so the docs and inputs stay in sync.
---
Duplicate comments:
In @.github/workflows/multi-platform-build-and-publish.yml:
- Line 113: Update the GitHub Actions workflow to disable CocoaPods by changing
the use_cocoapods setting from true to false; locate the use_cocoapods entry in
the multi-platform-build-and-publish workflow (look for the symbol
use_cocoapods) and set it to false so iOS lanes don’t attempt Pod operations
until Podfile/CocoaPods config is added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 684cd846-f106-4a8d-b8df-e22e43a84228
📒 Files selected for processing (1)
.github/workflows/multi-platform-build-and-publish.yml
| # WORKFLOW INPUTS: | ||
| # - release_type: 'internal' (default) or 'beta' | ||
| # - target_branch: Branch to use for release (default: 'development') | ||
| # - android_package_name: Name of Android module | ||
| # - ios_package_name: Name of iOS module | ||
| # - desktop_package_name: Name of desktop module | ||
| # - web_package_name: Name of web module | ||
| # - publish_android: Enable/disable Android Play Store publishing | ||
| # - build_ios: Enable/disable iOS build | ||
| # - publish_ios: Enable/disable iOS App Store publishing | ||
|
|
There was a problem hiding this comment.
Workflow input documentation block is stale.
Lines 44–47 describe publish_android, build_ios, and publish_ios, but those inputs are not defined under workflow_dispatch. This is misleading for release operators.
Proposed doc-only correction
-# - publish_android: Enable/disable Android Play Store publishing
-# - build_ios: Enable/disable iOS build
-# - publish_ios: Enable/disable iOS App Store publishing
+# - distribute_ios_firebase: Enable/disable iOS Firebase App Distribution
+# - distribute_ios_testflight: Enable/disable iOS TestFlight distribution
+# - distribute_ios_appstore: Enable/disable iOS App Store distribution🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/multi-platform-build-and-publish.yml around lines 37 - 47,
The workflow comment block in
.github/workflows/multi-platform-build-and-publish.yml is stale: it lists inputs
publish_android, build_ios, and publish_ios that are not defined under
workflow_dispatch; update the documentation to match the actual
workflow_dispatch inputs (remove the three obsolete entries or add the
corresponding workflow_dispatch inputs if they are intended features). Locate
the block containing the commented "WORKFLOW INPUTS" and the referenced symbols
publish_android, build_ios, publish_ios and either delete those lines or add the
missing inputs under workflow_dispatch so the docs and inputs stay in sync.
|
@techsavvy185 Please don't add usernames to commit messages. |
|
@biplab1 Actually I didn't add it myself, I just clicked apply suggestion and clicked on. I'll take care of it from the next time onwards. |
|
@techsavvy185 Can you please confirm about the Gemfile.lock - was it auto-generated or you had used it from the reference project? If it was auto-generated then please add it back. |
|
@biplab1 it wasn't autogenerated I edited it using the other project and saw your review on the Mifos mobile pr hence removed it. |
|
@techsavvy185 Please generate the Gemfile.Lock and add it in this PR. |
biplab1
left a comment
biplab1
left a comment
There was a problem hiding this comment.
Please revert these changes I had requested earlier since the cocoapods wasn't setup, however, there is another PR (#2654) in progress which aims to implement it. So, in order to avoid any conflicts, let's keep it as it was.
biplab1
left a comment
biplab1
left a comment
There was a problem hiding this comment.
Looks good to me. This can be merged.
niyajali
left a comment
niyajali
left a comment
There was a problem hiding this comment.
Recheck everything and each file from template and mifos-pay project and make sure
biplab1
changed the title
niyajali
left a comment
niyajali
left a comment
There was a problem hiding this comment.
the keystores/original_keystore.keystore file shouldn't be copied.
|
|
3 participants
Fixes - Jira-#714
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
New Features
Documentation
Chores