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

feat(ios): use Bundler for pods installation #1708

Merged

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Oct 4, 2022

Summary:

  • Using Bundler as a default way to handle dependencies on iOS

Test Plan:

  • yarn test
  • Possible error output, when other ruby version is used:

Screenshot 2022-10-04 at 11 42 28

  • Successful run:

Screenshot 2022-10-04 at 12 45 10

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Hi! Thank you so much for taking the time to implement this. I left a question and a very small suggestion, but the code looks good to me.
Waiting for the CLI maintainers to chime in.


async function runBundleInstall(loader: Loader) {
try {
loader.start('Installing Bundler');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this line does not appear in the successful run posted in the Test Plan? 🤔

Copy link
Contributor Author

@hoxyq hoxyq Oct 4, 2022

Choose a reason for hiding this comment

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

Why this line does not appear in the successful run posted in the Test Plan? 🤔

Looks like I need to make a separate loader.succeed call, will fix that

logger.error(error.stderr || error.stdout);

throw new Error(
'Looks like your iOS environment is not properly set. Please go to https://reactnative.dev/docs/next/environment-setup and follow the React Native CLI QuickStart for macOS and iOS.',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
'Looks like your iOS environment is not properly set. Please go to https://reactnative.dev/docs/next/environment-setup and follow the React Native CLI QuickStart for macOS and iOS.',
'Looks like your iOS environment is not properly set. Please go to https://reactnative.dev/docs/next/environment-setup and follow the React Native CLI QuickStart guide for macOS and iOS.',

Copy link
Collaborator

@adamTrz adamTrz left a comment

Choose a reason for hiding this comment

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

Great job! Thanks for contribution @hoxyq !

@hoxyq hoxyq force-pushed the use-bundler-for-cocoapods branch from 92a2d5c to 40dccc4 Compare October 4, 2022 11:46
@hoxyq
Copy link
Contributor Author

hoxyq commented Oct 4, 2022

Updated:

  • added logger.succeed call to fix appearing of Installing Bundler message in logs
  • fixed error message

@adamTrz adamTrz merged commit 49a945d into react-native-community:main Oct 4, 2022
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