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

Improve react-native-xcode.sh integration #5518

Closed
wants to merge 1 commit into from

Conversation

frantic
Copy link
Contributor

@frantic frantic commented Jan 24, 2016

Inspired by conversation in #5374, this PR improves react-native-xcode.sh:

  • No longer depends on global react-native binary
  • Gracefully handles missing node dependency and adds a new way to configure the path to node in non-standard installation environments

This is how the error looks like:
image

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @frantic, @hkjorgensen and @foghina to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 24, 2016
@frantic frantic force-pushed the fix-react-native-xcode branch from 045b574 to 08886b4 Compare January 24, 2016 21:16
@facebook-github-bot
Copy link
Contributor

@frantic updated the pull request.

@frantic
Copy link
Contributor Author

frantic commented Jan 25, 2016

cc @bestander @mkonicek

@frantic frantic force-pushed the fix-react-native-xcode branch from 08886b4 to 2d24dac Compare January 25, 2016 01:08
@facebook-github-bot
Copy link
Contributor

@frantic updated the pull request.

@mkonicek
Copy link
Contributor

Thanks so much! Don't have all the context but lgtm.

Could this issue be related somehow? #5523

@mkonicek
Copy link
Contributor

The error message is awesome! Great the path is configurable now, too!

@mkonicek
Copy link
Contributor

Feel free to shipit :)

@bestander
Copy link
Contributor

hammer01

@hkjorgensen
Copy link
Contributor

LGTM 👍

@frantic
Copy link
Contributor Author

frantic commented Jan 25, 2016

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/430209143850810/int_phab to review.

@ghost ghost closed this in d3e4414 Jan 25, 2016
mkonicek pushed a commit that referenced this pull request Jan 29, 2016
Summary:
Inspired by conversation in #5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes #5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
Inspired by conversation in facebook/react-native#5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes facebook/react-native#5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
Inspired by conversation in facebook/react-native#5374, this PR improves `react-native-xcode.sh`:

* No longer depends on global `react-native` binary
* Gracefully handles missing `node` dependency and adds a new way to configure the path to `node` in non-standard installation environments

This is how the error looks like:
![image](https://cloud.githubusercontent.com/assets/192222/12538882/3f9b5c3e-c29a-11e5-84fc-c7ccedf1c46a.png)
Closes facebook/react-native#5518

Reviewed By: svcscm

Differential Revision: D2861116

Pulled By: frantic

fb-gh-sync-id: 9a80eda6c844d066e34369b1cda503955171485b
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants