-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
dns: add verbatim
option to dns.lookup()
#14731
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -553,7 +553,7 @@ console.log('looking up nodejs.org...'); | |
|
||
const cares = process.binding('cares_wrap'); | ||
const req = new cares.GetAddrInfoReqWrap(); | ||
cares.getaddrinfo(req, 'nodejs.org', 4); | ||
cares.getaddrinfo(req, 'nodejs.org', 4, /* hints */ 0, /* verbatim */ true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could compare results to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That assumes python's implementation is exactly the same as node's, though. If it's not, or worse, not always, it's going to be a right pain in the neck to debug. If you still think it's a good idea, can you file a separate issue to hash out the details? |
||
|
||
req.oncomplete = function(err, domains) { | ||
assert.strictEqual(err, 0); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on preferring
verbatim: true
, but shouldn't we add a runtime deprecation of the old default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems like a separate question to "should this be an option". This is
semver-minor
(I think), and could be backported to v6.x. A runtime deprecation would bemajor
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation-only deprecation would be better, I think. We're not likely to get rid of the default, just change it. A runtime deprecation would be too noisy.