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

use default ports if empty/invalid (fixes #110) #111

Merged
merged 1 commit into from
Dec 3, 2015
Merged

use default ports if empty/invalid (fixes #110) #111

merged 1 commit into from
Dec 3, 2015

Conversation

cvan
Copy link
Contributor

@cvan cvan commented Dec 3, 2015

my use case is wanting the port to not start at 9966 by default

see #110 (comment) and #110 (comment)

let me know if there's any way I can make this change kosher

mattdesl added a commit that referenced this pull request Dec 3, 2015
use default ports if empty/invalid (fixes #110)
@mattdesl mattdesl merged commit a3483dd into mattdesl:master Dec 3, 2015
@mattdesl
Copy link
Owner

mattdesl commented Dec 3, 2015

Cool, I think this is alright since I've also been trying to find ways of using environment variables without relying on Unix-isms.

@yoshuawuyts
Copy link
Collaborator

Oh no, this introduces hard to debug bugs. E.g. "Why is my port 3000 if I'm passing a port variable?" Only after diving into budo source and logging values out will it be found that the port is not the one you thought you were setting, signalling a problem outside budo. budo should not do something unexpected but rather fail explicitly.

For simple use cases it might not be a problem, for more integrated solutions (who know what people use this for) this will break and bite people. 👎 we should revert this.

@mattdesl
Copy link
Owner

mattdesl commented Dec 3, 2015

Hmm. I agree this can cause surprise down the line.

Is there a way to do what @cvan is doing without relying on UNIX-only shell stuff? Maybe package.json config for env vars?

@yoshuawuyts
Copy link
Collaborator

I think the only reliable way of getting anything done on Windows is relying on libuv / Node itself. E.g. the solution posted about using JavaScript to work around this would be valid for Windows. Though it's clunky, it's reliable. Definitely better than swallowing errors.

@cvan
Copy link
Contributor Author

cvan commented Dec 4, 2015

No no no, this is introduces hard to debug bugs. E.g. "Why is my port 3000 if I'm passing a port variable?" Only after diving into budo source and logging values out will it be found that the port is not the one you thought you were setting, signalling a problem outside budo. budo should not do something unexpected but rather fail explicitly.

For simple use cases it might not be a problem, for more integrated solutions (who know what people use this for) this will break and bite people. 👎 we should revert this.

budo was actually doing exactly what I didn't expect it to do - force a default port to start at without my ability to change the default.

I expected it to work like http-server (where PORT=5000 http-server starts the server at http://0.0.0.0:5000/). But budo doesn't/didn't.

screenshot 2015-12-04 02 11 07

@yoshuawuyts
Copy link
Collaborator

I expected it to work like http-server

That's a separate from this issue; arguably it would make sense that budo can act based on a BUDO_PORT variable set, with --port taking priority over that. Would that resolve your issue?

What's currently going on, however, is that instead of handling an error budo goes off and does something it shouldn't.

@mattdesl
Copy link
Owner

mattdesl commented Dec 4, 2015

Ok, will revert this change back to how it was before.

mattdesl added a commit that referenced this pull request Jan 28, 2017
use default ports if empty/invalid (fixes #110)
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