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

doc: fix confusing example in dns.md #11022

Closed
wants to merge 1 commit into from
Closed

doc: fix confusing example in dns.md #11022

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, dns

Currently, the second example throws Error: getHostByAddr ENOTFOUND [IP]. Somebody even posts a question in stackoverflow.com for this very example recently.

Unfortunately, more neutral hostnames (like example.com or github.com) produce the same error. So I've replaced nodejs.org by google.com. If I should use something else, please, tell me.

The previous adjacent example is edited for consistency.

@nodejs-github-bot nodejs-github-bot added dns Issues and PRs related to the dns subsystem. doc Issues and PRs related to the documentations. lts-watch-v6.x labels Jan 26, 2017
@Trott
Copy link
Member

Trott commented Jan 26, 2017

I'd prefer we avoid corporate entities in our example URLs. Not enough to object to this landing, but enough to suggest some alternatives. (I tested these all and they work in the reverse lookup example in the StackOverflow question, at least.) Maybe use each of these in one example?:

  • iana.org
  • wikipedia.org
  • archive.org
  • iso.org

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jan 26, 2017

@Trott

Maybe use each of these in one example

Does it mean iteration over hostnames array?

@Trott
Copy link
Member

Trott commented Jan 26, 2017

Does it mean iteration over hostnames array?

I mean: There are four code examples changed in this PR. In one of those, use iana.org. In the next one, use wikipedia.org. And so on. Just a suggestion.

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jan 26, 2017

@Trott Sorry, there are two examples and two comments for them. So it could be only two hostnames. But these two examples illustrate 'functions belonging to two different categories', i.e. they are used in a somehow comparative way. Should we use different hostnames in them?

@Trott
Copy link
Member

Trott commented Jan 26, 2017

Oh, my mistake, I see now, yes, just two examples.

I don't think it matters much if you use the same hostname in the two examples or different hostnames in each. It was just an idea. Do whichever you think is better!

Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jan 26, 2017

@Trott I've used the archive.org in the second example as it has most similar input and output forms. Thank you for the suggestions!

@jasnell
Copy link
Member

jasnell commented Jan 27, 2017

Odd that it happens with example.org...

@vsemozhetbyt
Copy link
Contributor Author

@Trott , @gibfahn , @jasnell Could this be landed?

@gibfahn gibfahn self-assigned this Jan 31, 2017
@gibfahn
Copy link
Member

gibfahn commented Jan 31, 2017

@Trott does this LGTY?

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM

@gibfahn
Copy link
Member

gibfahn commented Jan 31, 2017

Landed in 110b7cf

@gibfahn gibfahn closed this Jan 31, 2017
gibfahn pushed a commit that referenced this pull request Jan 31, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: #11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@vsemozhetbyt vsemozhetbyt deleted the dns.md branch January 31, 2017 21:50
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 2, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: nodejs#11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: #11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: #11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: #11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
Currently, the example throws 'Error: getHostByAddr ENOTFOUND'
The previous example is edited for consistency.

PR-URL: #11022
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dns Issues and PRs related to the dns subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants