Conversation
silverwind
added
dns
silverwind
force-pushed
the
remove-EADNAME
branch
from
f76a9a4 to
ec44ea4
Compare
|
Fixed the commit title. |
|
From #3051 (comment), everything that actually uses or defines the misspelled variant biojs-vis-blast-0.1.5.tgz/node/lib/dns.js:323:exports.ADNAME = 'EADNAME';
cares-1.0.1.tgz/lib/cares.js:381:exports.ADNAME = 'EADNAME';
commonjs-everywhere-0.9.7.tgz/node/lib/dns.js:203:exports.ADNAME = 'EADNAME';
flush-all-0.1.1.tgz/node-v0.13/lib/dns.js:323:exports.ADNAME = 'EADNAME';
jsg-0.0.3.tgz/testdata/node_core_modules/dns.js:258:exports.ADNAME = 'EADNAME';
nice-http-0.1.0-alfa.tgz/src/dns.js:256:exports.ADNAME = 'EADNAME';
node-core-lib-0.11.11.tgz/dns.js:258:exports.ADNAME = 'EADNAME';
node-natives-0.10.25.tgz/dns.js:203:exports.ADNAME = 'EADNAME';
pezhu-0.0.0.tgz/Downloads/node-v0.9.11/lib/dns.js:202:exports.ADNAME = 'EADNAME';
pn-0.0.1.tgz/dns.js:7: ADNAME: { enumerable: true, value: dns.ADNAME },
portable-js-0.0.3.tgz/misc/io/dns.js:338:exports.ADNAME = 'EADNAME';
portable-js-0.0.3.tgz/misc/node/dns.js:325:exports.ADNAME = 'EADNAME';Only the pn module actually uses it (I just filed an issue there), all the others copy-paste LGTM for a major. There should be more explanation in the commit message details, though. |
Adds the documented but missing DNS error exports.BADNAME. This export has been there before but got lost in a 2012 commit that added more error codes. #3076 will remove the wrong error code exports.ADNAME. PR-URL: #3051 Fixes: #3050 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
silverwind
force-pushed
the
remove-EADNAME
branch
from
ec44ea4 to
24fe112
Compare
This error code export was mistakingly introduced in a 2012 commit which added more error codes. The correct export.BADNAME was added in nodejs#3051. Semver: Major PR-URL: nodejs#3051 Fixes: nodejs#3050
|
Updated commit details and rebased. |
|
LGTM |
|
Thanks, landed in 680dda8. |
|
sorry to have missed this, I would have voted for adding a deprecation warning on a getter for this instead of jumping straight to removal |
|
actually, I'm pretty sure this was done over a weekend and merged way too fast, particularly for a semver-major, can we be a bit more patient on these kinds of things, pretty please? |
|
@rvagg It's a semver-major just to be on the safe side. It was never useful or documented -- it's a simple mistype. See #3051 (it also had the start of this discussion). I can hardly imagine how removing it could break (or even change) anything in real code. If you still feel that there has to be a warning, it's not too late to introduce it. What do you think? Just explaining things here, I got the point about semver-major PRs. |
|
OK .. so I'm a little slow on this one, just to confirm, this is simply an exported string and was not used or referenced by anything else right? |
|
@rvagg exactly. These error exports look to be there for comparision purpose so one can do |
|
ok, thanks, I retract my comments then! |
Adds the documented but missing DNS error exports.BADNAME. This export has been there before but got lost in a 2012 commit that added more error codes. #3076 will remove the wrong error code exports.ADNAME. PR-URL: #3051 Fixes: #3050 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
This error code export was mistakingly introduced in a 2012 commit which added more error codes. The correct export.BADNAME was added in #3051. Semver: Major PR-URL: #3051 Fixes: #3050 Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
4 participants
This is the
semver-majorpart of #3051 which removes the nonexistant errorcodeEADNAME.