deps,debugger: move node-inspect into core#38161
Merged
Trott merged 12 commits intonodejs:masterfrom Apr 25, 2021
Merged
Conversation
Loading
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs: https://github.com/nodejs/node/discussions/36481
I named a lot of files/directories with
inspector-cliand I suspectinspect-cliordebugger-cli(or justdebuggerorinspect) would have all been better?Still some ESLint checks to re-enable in three files, but assuming tests pass everywhere, I think this is good enough to land and things can be adjusted with subsequent PRs.
Because I was porting over the tests, this found and fixed a small bug with
node inspectin Node.js 15.x. Specifically, callingscriptswould always include internal scripts, even withscripts(false). (Wouldn't mind reconsidering that API as that first argument is a boolean trap. But OMG not when first landing this thing.)Also there are some changes that landed in the
node-inspectrepo but never made it into a release. Those changes will need to be ported over. But that's the next PR. (Or maybe the lint fixes would be the next one.)