Skip to content

Conversation

joyeecheung
Copy link
Member

Instead of process.config.variables.v8_enable_inspector
which depends on the variable name in gyp files, or detecting
internalBinding('inspector').Connection.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 31, 2018
@joyeecheung
Copy link
Member Author

@jdalton
Copy link
Member

jdalton commented Dec 31, 2018

What I liked about process.config.variables.v8_enable_inspector is that it's also a user accessible way to detect if the inspector is enabled. Instead of moving to an internal solution it would be rad to expose this in a more user-friendly way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be !hasInspector

@joyeecheung
Copy link
Member Author

@jdalton I personally would not mind if there is an alias for internalBinding('config').hasInspector on process.features, but it would be a separate bikeshed about whether process.features should be deprecated in favor of something newer and shinier and what that might be ¯\(ツ)

Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.
@joyeecheung
Copy link
Member Author

Rebased and fixed test: https://ci.nodejs.org/job/node-test-pull-request/19910/

@joyeecheung
Copy link
Member Author

SmartOS ran out of memory. Resume: https://ci.nodejs.org/job/node-test-pull-request/19912/

@joyeecheung
Copy link
Member Author

Landed in b22c86e

@joyeecheung joyeecheung closed this Jan 4, 2019
joyeecheung added a commit that referenced this pull request Jan 4, 2019
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: #25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung
Copy link
Member Author

Opened #25343 about process.features

@addaleax
Copy link
Member

addaleax commented Jan 5, 2019

Should this be backported to v11.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: nodejs#25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: #25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: nodejs#25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
Instead of `process.config.variables.v8_enable_inspector`
which depends on the variable name in gyp files, or detecting
`internalBinding('inspector').Connection`.

PR-URL: nodejs#25291
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants