FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

dns.lookup return a string with no options by dcharbonnier · Pull Request #11334 · nodejs/node · GitHub

/ node Public

dns.lookup return a string with no options - #11334

Closed
dcharbonnier wants to merge 1 commit into
nodejs:masterfrom
dcharbonnier:patch-1
ConversationCommits (1)ChecksFiles changed
Closed

dns.lookup return a string with no options#11334
dcharbonnier wants to merge 1 commit into
nodejs:masterfrom
dcharbonnier:patch-1

Conversation

dcharbonnier commented Feb 13, 2017

Copy link
Copy Markdown
Contributor

If all is false (default) the return value is not an array, using a pluralized variable name is confusing

dns.lookup return a string with no options
1be79f0 
If all is false (default) the return value is not an array, using a pluralized variable name is confusing
nodejs-github-bot added dns Issues and PRs related to the dns subsystem. doc Issues and PRs related to the documentations. labels Feb 13, 2017
jasnell reviewed Feb 13, 2017
View reviewed changes
Comment thread doc/api/dns.md
dns.lookup('iana.org', (err, addresses, family) => {
console.log('addresses:', addresses);
dns.lookup('iana.org', (err, address, family) => {
console.log('address:', address);

jasnell Feb 13, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It would likely be worthwhile showing two examples, this one and one with all = true so that address becomes an Array.

dcharbonnier Feb 13, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this PR fix a mistake, not to enhance the documentation, I can do it but I'm scared that we end up on a very long debate on when to provide examples or not instead of a 5 seconds fix....

sam-github Feb 13, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@jasnell #11350

sam-github mentioned this pull request Feb 13, 2017
Merged
2 tasks
sam-github added a commit to sam-github/node that referenced this pull request Feb 16, 2017
doc: dns examples implied string args were arrays
642eec1 
Fix: nodejs#11334
PR-URL: nodejs#11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 20, 2017
doc: dns examples implied string args were arrays
010ae2e 
Fix: nodejs#11334
PR-URL: nodejs#11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
doc: dns examples implied string args were arrays
c3c874f 
Fix: #11334
PR-URL: #11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
doc: dns examples implied string args were arrays
f6ddf8f 
Fix: #11334
PR-URL: #11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
doc: dns examples implied string args were arrays
4ddab7f 
Fix: #11334
PR-URL: #11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
doc: dns examples implied string args were arrays
53d5002 
Fix: #11334
PR-URL: #11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
doc: dns examples implied string args were arrays
0c9ea4f 
Fix: #11334
PR-URL: #11350
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Trott commented Aug 3, 2017

Copy link
Copy Markdown
Member

I'd like this to land as-is. It has two approvals so it should be land-able.

Trott reopened this Aug 3, 2017

Trott commented Aug 3, 2017

Copy link
Copy Markdown
Member

@dcharbonnier Ooops, didn't notice that it was the OP that closed it. Uh, yeah, if you'd prefer to close it again, I won't re-open it.

Trott commented Aug 3, 2017

Copy link
Copy Markdown
Member

Rebased and there are no changes, so it looks like this change was handled (probably by another author?) in a different PR. Closing again. Sorry for the noise, everyone.

Trott closed this Aug 3, 2017
dcharbonnier mentioned this pull request Aug 3, 2017
Closed
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell left review comments
bnoordhuis bnoordhuis approved these changes
cjihrig cjihrig approved these changes
+1 more reviewer
sam-github sam-github left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dns Issues and PRs related to the dns subsystem. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL