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

Add warning when HOST environment variable is set (#3719) #3730

Merged
merged 3 commits into from Jan 14, 2018

Conversation

Projects
None yet
4 participants
@iansu
Copy link
Collaborator

commented Jan 10, 2018

To see the new warning message run HOST=8.8.8.8 npm run start.

Closes #3719

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2018

Can you please shorten the link?

Let's make the second log more along these lines:

`Learn more here: ${chalk.yellow('http://bit.ly/2mwWSwH')}`
if (process.env.HOST) {
console.log(
chalk.yellow(
"Attempting to bind to $HOST because it was specified. If this was unintentional, check that you haven't mistakenly set it in your shell."

This comment has been minimized.

Copy link
@gaearon

gaearon Jan 14, 2018

Member

Shouldn't we inline the HOST value in the message?

Also it's not 100% clear what "$HOST" is. (For example windows uses a different syntax for environment variables.) It is better to explicitly say "environment variable".

This comment has been minimized.

Copy link
@iansu

iansu Jan 14, 2018

Author Collaborator

What if it was something more like "Attempting to bind to HOST environment variable somehostname.example.com. If this was..."

@Timer Timer modified the milestones: 2.x, 1.0.18 Jan 14, 2018

@gaearon
Copy link
Member

left a comment

LGTM (haven't run it though)

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2018

Thanks!

@Timer Timer merged commit b86fe05 into facebook:master Jan 14, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2018

Adjusted the colors a bit:

@iansu

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2018

Looks good! 👍

@iansu iansu deleted the iansu:host-env-warning branch Jan 14, 2018

@iansu iansu restored the iansu:host-env-warning branch Jan 14, 2018

@iansu iansu deleted the iansu:host-env-warning branch Jan 14, 2018

@gaearon gaearon referenced this pull request Jan 15, 2018

Merged

Changelog for 1.1.0 #3795

tabrindle added a commit to tabrindle/create-react-app that referenced this pull request Jan 18, 2018

Add warning when HOST environment variable is set (facebook#3730)
* Add warning when HOST environment variable is set (facebook#3719)

* Improve HOST environment variable warning message

* Adjust text and message

Closes facebook#3719

@renovate renovate bot referenced this pull request Feb 2, 2018

Open

fix(deps): update dependency react-scripts to v1.1.5 #17

0 of 1 task complete

Pavek pushed a commit to Pavek/create-react-app that referenced this pull request Jul 10, 2018

Add warning when HOST environment variable is set (facebook#3730)
* Add warning when HOST environment variable is set (facebook#3719)

* Improve HOST environment variable warning message

* Adjust text and message

Closes facebook#3719

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Aug 14, 2018

Add warning when HOST environment variable is set (facebook#3730)
* Add warning when HOST environment variable is set (facebook#3719)

* Improve HOST environment variable warning message

* Adjust text and message

Closes facebook#3719

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.