Skip to content
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

deps: Add node-inspect 1.10.6 #11869

Closed
wants to merge 2 commits into from
Closed

Conversation

jkrems
Copy link
Contributor

@jkrems jkrems commented Mar 15, 2017

This updates the bundled node-inspect to 1.10.6.

Highlights

  • node --debug-port=1234 inspect respects the custom port.
  • Test stability improvements on various platforms.

Compare: nodejs/node-inspect@v1.10.4...v1.10.6

Rendered Changelog

1.10.6

  • chore: Fix usage text for embedded mode - @addaleax #20
    • b0779f5 chore: Fix usage text for embedded mode
  • print 'ok' after connection - @ofrobots #25
    • 2a47125 fix: print 'ok' after connection
  • Make autocompletion in REPL work - @aqrln #28
    • ccab737 fix: Make autocompletion in REPL work
  • Remove console.error() statement - @aqrln #30
    • 032b045 style: Remove console.error() statement
  • Take --debug-port into account - @jkrems #26
    • 054d4b1 fix: Take --debug-port into account
  • Delay run until breakpoints are restored - @jkrems #34
    • 802b88c fix: Delay run until breakpoints are restored
    • 2b93173 fix: Use single string for paused notice
    • b4d5ee2 fix: Work around inconsistent handling of strict directive
    • f6ccfc7 fix: Only restart after port is free
    • 8b101bf test: Skip exact match on AIX
  • a4e4b6f chore: Fix repo info in package.json

1.10.5

  • docs: minor edits to governance docs - @joshgav #17
    • a70fe04 docs: minor edits to governance docs
Checklist
  • make test-node-inspect passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

node-inspect

This updates the bundled `node-inspect` to 1.10.6.

Highlights:

* `node --debug-port=1234 inspect` respects the custom port.
* Test stability improvements on various platforms.

Compare: nodejs/node-inspect@v1.10.4...v1.10.6
@jkrems
Copy link
Contributor Author

jkrems commented Mar 15, 2017

Previous PR: #10187

Changeset generated via:

rm -rf deps/node-inspect node-inspect-* && curl -sSL "https://github.com/nodejs/node-inspect/archive/v1.10.6.tar.gz" | tar -xzvf - && mv node-inspect-* deps/node-inspect

@jkrems
Copy link
Contributor Author

jkrems commented Mar 15, 2017

One test fails because of https://github.com/nodejs/node/pull/6171/files#r106263519 - <enter> is swallowed inside of repl.

@@ -81,7 +81,9 @@

} else if (process.argv[1] === 'inspect') {
// Start the debugger agent
NativeModule.require('node-inspect/lib/_inspect').start();
process.nextTick(function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hack appear in multiple places in bootstrap. What I saw before adding it: The pingPort function calls net.connect and setTimeout. Unless the latter was removed or moved to the next tick, none of the connect events would ever fire.

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Rubber-stamp-ish LGTM (lib: might be a better subsystem for the second commit, though)

CI: https://ci.nodejs.org/job/node-test-commit/8488/

@jkrems
Copy link
Contributor Author

jkrems commented Mar 15, 2017

s/deps:/lib: for that second commit message. Guess I went with deps: because I was still thinking about merging the two commits while writing the commit message. Thanks for clarifying! 👍

@jkrems
Copy link
Contributor Author

jkrems commented Mar 15, 2017

Since make test-node-inspect doesn't pass on master right now, I backed that bullet point out of the issue description (working on making them pass again in parallel).

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Rubber-stamp LGTM if CI is ✅

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Rubber stamp LGTM here as well. Tried it out and it appears to work as expected :-)

@jkrems
Copy link
Contributor Author

jkrems commented Mar 16, 2017

I think my force-push to reword the commit is why those CI builds failed (Could not checkout 7f0e8337eaf1198a9f23a2de25d2b9418800f84e).

@jasnell
Copy link
Member

jasnell commented Mar 17, 2017

@jkrems
Copy link
Contributor Author

jkrems commented Mar 17, 2017

🍏

jasnell pushed a commit that referenced this pull request Mar 17, 2017
This updates the bundled `node-inspect` to 1.10.6.

Highlights:

* `node --debug-port=1234 inspect` respects the custom port.
* Test stability improvements on various platforms.

Compare: nodejs/node-inspect@v1.10.4...v1.10.6

PR-URL: #11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 17, 2017
PR-URL: #11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 17, 2017

Landed in b57f9ee...7042631

@jasnell jasnell closed this Mar 17, 2017
@jkrems jkrems deleted the node-inspect-1.10.6 branch March 17, 2017 18:09
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
PR-URL: nodejs#11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
This updates the bundled `node-inspect` to 1.10.6.

Highlights:

* `node --debug-port=1234 inspect` respects the custom port.
* Test stability improvements on various platforms.

Compare: nodejs/node-inspect@v1.10.4...v1.10.6

PR-URL: nodejs#11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
This updates the bundled `node-inspect` to 1.10.6.

Highlights:

* `node --debug-port=1234 inspect` respects the custom port.
* Test stability improvements on various platforms.

Compare: nodejs/node-inspect@v1.10.4...v1.10.6

PR-URL: nodejs#11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
PR-URL: nodejs#11869
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
cjihrig added a commit that referenced this pull request Mar 21, 2017
Notable changes:

* deps: Add node-inspect 1.10.6 (Jan Krems) #11869
* inspector: proper WS URLs when bound to 0.0.0.0 (Eugene Ostroukhov) #11850
* tls: fix segfault on destroy after partial read. (Ben Noordhuis) #11898

PR-URL: #11941
cjihrig added a commit that referenced this pull request Mar 21, 2017
Notable changes:

* deps: Add node-inspect 1.10.6 (Jan Krems) #11869
* inspector: proper WS URLs when bound to 0.0.0.0 (Eugene Ostroukhov) #11850
* tls: fix segfault on destroy after partial read. (Ben Noordhuis) #11898

PR-URL: #11941
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants