-
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
test: remove usage of process.binding()
#26304
Conversation
@addaleax sadly an error occured when I tried to trigger a build :( |
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.
RS-ish LGTM
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.
LGTM. AFAIK there is a conflicting PR open for a while.
Refs: #24952 |
@BridgeAR Yeah, thanks. I’ll try to land that one first. |
c706a3b
to
9a9b038
Compare
Prefer `internalBinding` or other equivalents over `process.binding()` (except in tests checking `process.binding()` itself).
9a9b038
to
8c99173
Compare
Rebased, new CI: https://ci.nodejs.org/job/node-test-pull-request/21296/ |
c12a713
to
4e3990b
Compare
Fixed one more error that just (?) showed up, CI: https://ci.nodejs.org/job/node-test-pull-request/21297/ |
Landed in aec3447 |
Prefer `internalBinding` or other equivalents over `process.binding()` (except in tests checking `process.binding()` itself). PR-URL: nodejs#26304 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Prefer `internalBinding` or other equivalents over `process.binding()` (except in tests checking `process.binding()` itself). PR-URL: nodejs#26304 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Prefer `internalBinding` or other equivalents over `process.binding()` (except in tests checking `process.binding()` itself). PR-URL: #26304 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This paves the way for adding
--pending-deprecation
support forprocess.binding()
, see e.g. addaleax/node@8e6577d (which could land after this, but would likely be semver-major).Prefer
internalBinding
or other equivalents overprocess.binding()
(except in tests checking
process.binding()
itself).Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes