Skip to content
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: fastlane updates to build sample app #137

Merged
merged 5 commits into from
Aug 13, 2024

Conversation

mrehan27
Copy link
Contributor

@mrehan27 mrehan27 commented Aug 9, 2024

part of MBL-420

Changes

  • Updated Gemfile and Gemfile.lock to include required dependencies
  • Updated Appfile, Gymfile, Matchfile and Pluginfile to match values required by Fastlane to build sample apps on CI
  • Broke down Fastfile into following smaller files
    • build_helper.rb-> Helps building and uploading Android and iOS builds
    • github_helper.rb -> Helps fetching Github attributes conveniently (mostly code movement only)
    • version_helper.rb -> Helps updating app and SDK versions on CI

@mrehan27 mrehan27 self-assigned this Aug 9, 2024
@mrehan27 mrehan27 force-pushed the rehan/mbl-420-fastlane-updates branch from 8403040 to 59557c4 Compare August 9, 2024 15:57
require 'json'

# Fastlane reacts differently by when it gets executed in GitHub Actions.
class GitHub
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is moved to github_helper.rb so we can reuse in multiple repos later


# example for main builds: `bundle exec fastlane android deploy_app version:1.0.0"`
# example for develoment builds (pull request, push): `bundle exec fastlane deploy_app`
lane :deploy_app do |values|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of this is removed and some moved and updated in build_helper.rb and version_helper.rb

@mrehan27 mrehan27 requested a review from a team August 9, 2024 16:01

# Lane to update Flutter app version
lane :update_flutter_sdk_version do |options|
project_path = options[:project_path] || File.join(Dir.pwd, '../../..')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is File.join(Dir.pwd, '../../..') safe to use, or do you think it will result in bugs if someone moves a file and something breaks?

I am OK with the code, but I wonder if maybe a UI.message( right above would help if it does break, we can fix it quickly.

Suggested change
project_path = options[:project_path] || File.join(Dir.pwd, '../../..')
project_path = options[:project_path] || File.join(Dir.pwd, '../../..')
UI.message("updating version for files in path....")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, it can surely break if the file is moved. To help with this, there is already a message just below this line:

UI.message("Updating sdk version to #{version_name} in #{project_path}")

Are you suggesting adding another message, or was it just missed?


# Lane to update Flutter app version
lane :update_flutter_app_version do |options|
project_path = options[:project_path] || File.join(Dir.pwd, '..')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above with use of File.join(Dir.pwd, '..')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a message for this one too

UI.message("Updating app versions to #{new_version} in #{project_path}")

apps/fastlane/helpers/build_helper.rb Outdated Show resolved Hide resolved
UI.important(find_firebase_app_id(app_identifier: app_package_name))

# Build release APK using Flutter CLI
sh("flutter build apk --release")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest we build an APK compared to AAB?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider using AAB as well, but then switched to APK because I think it will be easier for anyone to download from Firebase and install the APK if needed, especially for non-registered devices (e.g., emulators).

@levibostian
Copy link
Contributor

Great job!

Base automatically changed from rehan/mbl-420-sample-app-fixes to main August 13, 2024 11:00
@mrehan27 mrehan27 merged commit 603a593 into main Aug 13, 2024
3 of 4 checks passed
@mrehan27 mrehan27 deleted the rehan/mbl-420-fastlane-updates branch August 13, 2024 11:21
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.

3 participants