-
Notifications
You must be signed in to change notification settings - Fork 30k
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: synchronize resolve() docs and code #13090
Conversation
Sole test failure is an unrelated known flaky with a fix pending. |
/ping @nodejs/ctc This is ready to land except that it needs another signoff from a CTC member because it is |
Can use a more descriptive commit title? I wouldn't know what this is about if it were in the changelog like it is. |
I'm open to suggestions that are less than 50 characters. :-D |
49 chars: |
Synchronize the argument list for `dns.resolve()` with what's in the documentation. Improve the error for a bad `rrtype` to be a `TypeError` rather than an `Error`.
Rebased, updated commit message. New CI: https://ci.nodejs.org/job/node-test-pull-request/8352/ |
|
Synchronize the argument list for `dns.resolve()` with what's in the documentation. Improve the error for a bad `rrtype` to be a `TypeError` rather than an `Error`. PR-URL: nodejs#13090 Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Landed in 758a17f |
It's fine if this has to wait for Node 9.0.0 to be released, but for the record, it is the tiniest of semver majors (change in type from |
Synchronize the argument list for
dns.resolve()
with what's in thedocumentation.
Improve the error for a bad
rrtype
to be aTypeError
rather than anError
.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
dns