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

[Linux/OSX] Add missing "--terminal" argument to run-android #20584

Closed
wants to merge 2 commits into from

Conversation

jamsch
Copy link
Contributor

@jamsch jamsch commented Aug 8, 2018

Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes #18122

Test Plan:

None

Release Notes:

[CLI] [BUGFIX] [local-cli/runAndroid/runAndroid.js] - Add missing "--terminal" argument

Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository. 

This commit now allows the user to specify their custom terminal as an argument as per the following:

    react-native run-android --terminal=x-terminal-emulator

Additionally the child process "detached" option has been set to false if no terminal has been specified which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process. 

Fixes facebook#18122
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 8, 2018
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@jamsch jamsch changed the title Add missing "--terminal" argument to run-android [Linux/OSX] Add missing "--terminal" argument to run-android Aug 9, 2018
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 9, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Aug 9, 2018
@hramos
Copy link
Contributor

hramos commented Aug 20, 2018

Re-importing. This failed earlier, because the PR was updated after it was approved but before it could be imported.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 20, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was closed by @jamsch in d846696.

Once this commit is added to a release, you will see the corresponding version tag below the description at d846696. If the commit has a single master tag, it is not yet part of a release.

@facebook facebook locked as resolved and limited conversation to collaborators Aug 20, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 20, 2018
hramos pushed a commit that referenced this pull request Aug 22, 2018
Summary:
Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes #18122
Pull Request resolved: #20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
aleclarson pushed a commit to aleclarson/react-native that referenced this pull request Sep 16, 2018
Summary:
Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes facebook#18122
Pull Request resolved: facebook#20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
Commit 79e498b7fbf19ab50bb73697602910fb827ff928 claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes #18122
Pull Request resolved: facebook/react-native#20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. Import Failed labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes facebook#18122
Pull Request resolved: facebook#20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes facebook#18122
Pull Request resolved: facebook#20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JS Server Never Terminates
4 participants