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

Inconsistent documentation: hostname vs host #20892

Closed
apapirovski opened this issue May 22, 2018 · 17 comments
Closed

Inconsistent documentation: hostname vs host #20892

apapirovski opened this issue May 22, 2018 · 17 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.

Comments

@apapirovski
Copy link
Member

apapirovski commented May 22, 2018

The Node.js documentation is currently rather freeform when it comes to distinguishing between hostname and host. There are arguments and properties named host that refer to hostname and their description makes as much clear. There are also arguments and properties that refer to host in both the name and the description, despite actually intending to represent the hostname.

This can without any doubt create confusion for end users but also for contributors. For a recent example see: #20875 or #20493

I think it would be nice if we could slowly update the the documentation and code to be more clear about this. While we cannot change property names on options objects, argument names can certainly be changed and documentation can be updated.

Some example entries:

https://nodejs.org/dist/latest-v10.x/docs/api/tls.html#tls_tls_checkserveridentity_host_cert
https://nodejs.org/dist/latest-v10.x/docs/api/tls.html#tls_tls_connect_port_host_options_callback
https://nodejs.org/dist/latest-v10.x/docs/api/tls.html#tls_tls_connect_options_callback
https://nodejs.org/dist/latest-v10.x/docs/api/net.html#net_server_listen_options_callback

@apapirovski apapirovski added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels May 22, 2018
@apapirovski apapirovski changed the title Inconsistent documentation: Inconsistent documentation: hostname vs host May 22, 2018
@apapirovski
Copy link
Member Author

@nodejs/documentation @nodejs/http @nodejs/http2

@mcollina
Copy link
Member

Definitely 👍 . I think this would be a good source of tasks for a code&learn.

@styfle
Copy link
Member

styfle commented May 22, 2018

I agree, this is a source of confusion for myself.

This ASCII Chart does a good job of explaining the difference.

I even found a Chrome Bug after reading through the Node.js docs regarding hostname assignment.

All that to say, I agree with this change 👍

@davisokoth
Copy link
Contributor

Interested in picking this up...

@apapirovski
Copy link
Member Author

@davisokoth let me know if you need any guidance. There are a few links in the OP to documentation mistakes but it would be nice to also update some of the variable names in our code. Feel free to also open a PR just with one of those documentation changes — we don't have to change everything all at once.

@benjipelletier
Copy link

Also interested in contributing to this

davisokoth added a commit to davisokoth/node that referenced this issue May 24, 2018
Update reference to read `hostname` instead of `host` for consistency.
Fixes nodejs#20892 partially

Also update function signature to use `hostname` rather than `host`
apapirovski pushed a commit that referenced this issue Jun 1, 2018
Update reference to read `hostname` instead of `host` for consistency.

Also update function signature to use `hostname` rather than `host`

PR-URL: #20933
Refs: #20892
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@lliepert
Copy link
Contributor

lliepert commented Jun 2, 2018

Also interested in contributing to this if help is still needed

@UlisesGascon
Copy link
Member

I am also interested in contributing...

targos pushed a commit to targos/node that referenced this issue Jun 6, 2018
Update reference to read `hostname` instead of `host` for consistency.

Also update function signature to use `hostname` rather than `host`

PR-URL: nodejs#20933
Refs: nodejs#20892
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@thatshailesh
Copy link
Contributor

@apapirovski anything left to be done or can you please point other issue to work on?

@sagirk
Copy link
Contributor

sagirk commented Jun 7, 2018

I'm working on doc/api/net.md and the corresponding lib/net.js.

targos pushed a commit that referenced this issue Jun 7, 2018
Update reference to read `hostname` instead of `host` for consistency.

Also update function signature to use `hostname` rather than `host`

PR-URL: #20933
Refs: #20892
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Backport-PR-URL: #21172
targos pushed a commit that referenced this issue Jun 13, 2018
Update reference to read `hostname` instead of `host` for consistency.

Also update function signature to use `hostname` rather than `host`

PR-URL: #20933
Refs: #20892
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Backport-PR-URL: #21172
@graezzon
Copy link

I'm interested in getting involved in this project. Thanks guys.

@apapirovski
Copy link
Member Author

@thatshailesh @sagirk @graezzon There's definitely more to be done. Some of the links in the OP haven't been updated and there are likely more instances of this in the codebase. Just look for any irregularities of hostname vs host usage, in particular cases where the term is used interchangeably to refer to the same thing.

@sagirk
Copy link
Contributor

sagirk commented Jun 20, 2018

@apapirovski I ran a project-wide search and found a few instances of this issue in

  • doc/api/tls.md and the corresponding lib/tls.js
  • doc/api/net.md and the corresponding lib/net.js (taken; I'm working on this)
  • doc/api/dns.md and the corresponding lib/dns.js

@sagirk
Copy link
Contributor

sagirk commented Jun 20, 2018

@benjipelletier @lliepert @UlisesGascon @thatshailesh @graezzon Take a look at one of the files listed above and claim it in this thread so we know who is working on what.

@UlisesGascon
Copy link
Member

Thanks @sagirk! I am working now in doc/api/dns.md and lib/dns.js. I can take tls too after that if nobody else does ;-)

@thatshailesh
Copy link
Contributor

Hi All
I will be taking doc/api/tls.md and lib/tls.js
Thanks @sagirk

thatshailesh added a commit to thatshailesh/node that referenced this issue Jun 21, 2018
Updated error messages and function arguments
Refs: nodejs#20892
@yokanand
Copy link

Hi guys, I'm late to join. I'm interested to contribute as well. Can I get started. Are there any open tasks to pick up?

shakeelmohamed pushed a commit to shakeelmohamed/node that referenced this issue Oct 12, 2018
@Trott Trott closed this as completed in beb0f03 Oct 21, 2018
jasnell pushed a commit that referenced this issue Oct 21, 2018
Fixes: #20892
PR-URL: #23572
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Nov 26, 2018
Fixes: #20892
PR-URL: #23572
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this issue Nov 28, 2018
Fixes: #20892
PR-URL: #23572
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this issue Nov 29, 2018
Fixes: #20892
PR-URL: #23572
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.
Projects
None yet