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

Minor fixes for setup_native.sh #1180

Merged
merged 6 commits into from
Dec 8, 2024
Merged

Conversation

catarial
Copy link
Contributor

The current setup script does not allow only performing a setup for ios or android. This change would fix that by just requiring them to be explicitly set instead of trying to do them both if none are set.

The script now requires brew if the correct ruby installation is not found, regardless of OSX version.

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.14%. Comparing base (4596d0c) to head (6dd71f1).
Report is 107 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1180   +/-   ##
=======================================
  Coverage   30.14%   30.14%           
=======================================
  Files         118      118           
  Lines        5172     5172           
  Branches     1110     1158   +48     
=======================================
  Hits         1559     1559           
+ Misses       3611     3609    -2     
- Partials        2        4    +2     
Flag Coverage Δ
unit 30.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

I would suggest simplifying further and just always installing both iOS and android dependencies. We can also unify the two separate CI/CD builds into one - e.g. osx-native-install.

setup/setup_native.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

LGTM! And I can test it out now on my new laptop as well!

@shankari shankari merged commit 3ecb3c5 into e-mission:master Dec 8, 2024
8 checks passed
shankari added a commit to shankari/e-mission-phone that referenced this pull request Dec 21, 2024
In e-mission#1180 we unified the setup
scripts so that we don't have separate android and iOS setup. This is because
the plugins include both, so installing the plugins will fail unless the
dependencies are installed, which, in turn, depends on having cocoapods installed

6dd71f1 fixed all the other github action
scripts, but left this one out since it wasn't triggered
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks completed
Development

Successfully merging this pull request may close these issues.

2 participants