Skip to content

Conversation

@sam-github
Copy link
Contributor

It does not affect dns.lookup().

cf. #25560

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added dns Issues and PRs related to the dns subsystem. doc Issues and PRs related to the documentations. labels Jan 18, 2019
@sam-githubsam-githubforce-pushed the doc-clarify-setservers branch from 5a91f8d to ccec12aCompareJanuary 18, 2019 18:12
doc/api/dns.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: unneeded empty line?

@sam-githubsam-githubforce-pushed the doc-clarify-setservers branch from ccec12a to 905a392CompareJanuary 23, 2019 22:44
@sam-github
Copy link
ContributorAuthor

Also, calling dns.setServers() should always be okay in recent versions of Node. Calling resolver.setServers() is also okay, but will yield an error if there are active queries.

I haven't changed this text, and don't want to get into it here. I'll add it to my todo list. Can you reference the PR that did this? It'll need YAML change markers. If you don't know, I'll track it down some time.

The structure of the docs are confusing. The Resolver class refers to the global docs, but it really be the other way around, the Resolver methods should be documented, and the globals should state that there is a global Resolver. An astute reader could guess that its implemented like this, but it isn't clearly stated.

@addaleax
Copy link
Member

I think the relevant PRs were #14891 (error out when active queries are on the resolver + change global default resolver when calling dns.setServers()) and, if you want, #14518 (adding the Resolver class).

It does not affect dns.lookup().
@sam-githubsam-githubforce-pushed the doc-clarify-setservers branch from 905a392 to d003bcdCompareJanuary 24, 2019 22:10
@sam-github
Copy link
ContributorAuthor

@sam-github
Copy link
ContributorAuthor

Landed in 0ce615c

@sam-githubsam-github deleted the doc-clarify-setservers branch January 25, 2019 18:18
addaleax pushed a commit that referenced this pull request Jan 28, 2019
It does not affect dns.lookup(). PR-URL: #25570 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@targostargos mentioned this pull request Jan 29, 2019
BethGriggs pushed a commit that referenced this pull request Apr 29, 2019
It does not affect dns.lookup(). PR-URL: #25570 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@BethGriggsBethGriggs mentioned this pull request May 1, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
It does not affect dns.lookup(). PR-URL: #25570 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
It does not affect dns.lookup(). PR-URL: #25570 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dnsIssues and PRs related to the dns subsystem.docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@sam-github@nodejs-github-bot@addaleax@lpinca@vsemozhetbyt@szmarczak