-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
src: remove process.binding('config').debugOptions
#25999
Conversation
59a2b71
to
f4e4da9
Compare
`process.binding('config').debugOptions`, which contains the initial values of parsed debugger-related CLI options, has been used for internal testing. This patch removes them and uses `internal/options` to query the values in the tests instead.
f4e4da9
to
26d3abd
Compare
Ping @nodejs/process @addaleax can I have some review please? |
CI is green … Should this be labelled |
@addaleax We can label it don't land on v11 on the safe side, although the properties exposed here are not even really useful for non-test use cases as they only capture the parsed CLI options, and are not in sync with the actual state of the process. e.g. if someone wants to know about the port they are more likely to use the documented |
Fresh CI: https://ci.nodejs.org/job/node-test-pull-request/20866/ (✔️) |
Landed in b200a46. |
`process.binding('config').debugOptions`, which contains the initial values of parsed debugger-related CLI options, has been used for internal testing. This patch removes them and uses `internal/options` to query the values in the tests instead. PR-URL: nodejs#25999 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
process.binding('config').debugOptions
, which contains the initialvalues of parsed debugger-related CLI options, has been used for
internal testing. This patch removes them and uses
internal/options
to query the values in the tests instead.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes