-
Notifications
You must be signed in to change notification settings - Fork 22
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
chore: build primary sample app #847
base: main
Are you sure you want to change the base?
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
SDK binary size reports 📊SDK binary size of this PR
SDK binary size diff report between this PR and the main branch
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #847 +/- ##
==========================================
+ Coverage 63.74% 64.98% +1.23%
==========================================
Files 153 153
Lines 6294 6294
==========================================
+ Hits 4012 4090 +78
+ Misses 2282 2204 -78 ☔ View full report in Codecov by Sentry. |
fi | ||
|
||
# Export the groups as an environment variable | ||
echo "firebase_distribution_groups=$(IFS=','; echo "${distribution_groups[*]}")" >> $GITHUB_ENV |
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.
I tested final value with dummy input, and it seems to be working well.
@@ -67,13 +67,37 @@ jobs: | |||
steps: | |||
- uses: actions/checkout@v4 | |||
|
|||
- name: Set Default Firebase Distribution Groups | |||
run: | |
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.
I think we can extract the variable initialization from run
to env
, something like
- name: Set Firebase Distribution Groups
shell: bash
env:
# Distribution group constants
ALL_BUILDS_GROUP: all-builds
FEATURE_BUILDS_GROUP: feature-branch
STABLE_BUILDS_GROUP: next
# Input variables
IS_PRIMARY_APP: ${{ matrix.apn-or-fcm == 'APN' }}
CURRENT_BRANCH: ${{ github.ref }}
run: |
# Initialize array with default group
distribution_groups=("$ALL_BUILDS_GROUP")
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.
I personally find reading bash logic in Github actions steps to make the file harder to read and navigate.
Would it make things simpler if we pass which app is being built to Fastlane and keep the logic that decides which groups to publish to in the get_build_test_groups
lane?
- We can send params
isPrimaryApp
and whatever else is needed
This will also allow us to share the logic as we used to between repos.
part of: MBL-710
Changes
Build Sample Apps
action to push only primary app to relevant channelsget_build_test_groups
lane to decouple from GitHub and accept arguments for passing Firebase distribution groups when distributing sample appsPlease refer to linked ticket for detailed expectations regarding channel distribution.