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

Serve on all interfaces by default #39

Merged
merged 1 commit into from
Nov 9, 2017
Merged

Conversation

demurgos
Copy link
Contributor

@demurgos demurgos commented Nov 9, 2017

This commit changes the default value of the --ip option to use
Node's internal resolution. It means that it will try to bind to
the unspecified IPv6 address :: if available or else fall back to the
unspecified IPv4 address 0.0.0.0. In most systems, :: also binds
to 0.0.0.0 which effectively allows to listen on all interfaces.

Closes #35

@coveralls
Copy link

coveralls commented Nov 9, 2017

Coverage Status

Coverage increased (+0.2%) to 94.545% when pulling 22fe933 on demurgos:issue-35 into 7b972cb on hexojs:master.

lib/server.js Outdated
@@ -7,11 +7,12 @@ var Promise = require('bluebird');
var format = require('util').format;
Copy link
Contributor

@JLHwung JLHwung Nov 9, 2017

Choose a reason for hiding this comment

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

Are format unused now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I switched to the more reliable require('url').format and forgot to remove the format import. I've seen the Travis error: I'll push an update.

@coveralls
Copy link

coveralls commented Nov 9, 2017

Coverage Status

Coverage increased (+0.2%) to 94.495% when pulling 586e8d6 on demurgos:issue-35 into 7b972cb on hexojs:master.

This commit changes the default value of the `--ip` option to use
Node's internal resolution. It means that it will try to bind to
the unspecified IPv6 address `::` if available or else fall back to the
unspecified IPv4 address `0.0.0.0`. In most systems, `::` also binds
to `0.0.0.0` which effectively allows to listen on all interfaces.

Closes hexojs#35
@coveralls
Copy link

coveralls commented Nov 9, 2017

Coverage Status

Coverage increased (+0.2%) to 94.495% when pulling 5efada5 on demurgos:issue-35 into 7b972cb on hexojs:master.

@demurgos
Copy link
Contributor Author

demurgos commented Nov 9, 2017

The Appveyor issue is due to a version mismatch with Node 9.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 9, 2017

Rubberstamp LGTM.

@demurgos
Copy link
Contributor Author

demurgos commented Nov 9, 2017

Any idea about Appveyor? I am not familiar with it.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 9, 2017

@demurgos appveyor complains simply because npm 5.5.1 does not officially support nodejs 9.x. As long as test on 8.x is successful, it is safe to merge.

Copy link
Member

@NoahDragon NoahDragon left a comment

Choose a reason for hiding this comment

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

Nice work. LGTM.

@demurgos demurgos merged commit dd31ccb into hexojs:master Nov 9, 2017
@demurgos demurgos deleted the issue-35 branch November 9, 2017 15:41
@demurgos
Copy link
Contributor Author

demurgos commented Nov 9, 2017

Since it seems fine, I'm merging it. Still, I quickly checked for the Appveyor issue: npm does not seem to use any engine dependency so it's a bit weird.

demurgos added a commit to demurgos/hexo-server that referenced this pull request Dec 22, 2017
This commit is a fix following the PR hexojs#39. This PR changed the
default behavior of the server to listen on all interfaces (both
IPv4 and IPv6). This commit was missing the update of the default
configuration, causing the server to still listen only to IPv4 by
default. The current commit fixes this error by using `undefined`
(Node's default) for the IP.

- See hexojs#35
- See hexojs#39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants