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.

Sign up for GitHub

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

Jump to bottom

backport: debugger: make listen address configurable #9692

Closed
wants to merge 1 commit into from
Closed

backport: debugger: make listen address configurable #9692

wants to merge 1 commit into from

Conversation

Qard
Copy link
Member

@Qard Qard commented Nov 19, 2016 •
edited
Loading

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)
  • debugger
Description of change

This is a backport of:

/cc @bnoordhuis @thealphanerd @nodejs/diagnostics

@bnoordhuis @Qard
debugger: make listen address configurable
868898a 
`--debug=1.2.3.4:5678` and `--debug=example.com:5678` are now accepted,
likewise the `--debug-brk` and `--debug-port` switch.  The latter is
now something of a misnomer but it's undocumented and for internal use
only so it shouldn't matter too much.

`--inspect=1.2.3.4:5678` and `--inspect=example.com:5678` are also
accepted but don't use the host name yet; they still bind to the
default address.

Fixes: nodejs#3306
PR-URL: nodejs#3316
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. debugger labels Nov 19, 2016
@mscdex
Copy link
Contributor

mscdex commented Nov 19, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/4905/

@Qard
Copy link
Member Author

Qard commented Nov 19, 2016

As far as I can tell, the freebsd failure is unrelated. Anyone know what that's about?

https://ci.nodejs.org/job/node-test-commit-freebsd/5319/nodes=freebsd11-x64/console

@Qard
Copy link
Member Author

Qard commented Nov 19, 2016

By the way, this removes the inspector-specific stuff from #3316. As far as I can tell, the final inspector API PR (#9028) is not getting backported, so that might be fine.

bnoordhuis
bnoordhuis approved these changes Nov 19, 2016
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this.

@MylesBorins MylesBorins added land-on-v4.x semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 22, 2016
@MylesBorins
Copy link
Contributor

landed in 0624e51

Thanks @Qard

@Qard Qard deleted the backport-3316-to-v4.x branch August 11, 2021 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Qard @mscdex @MylesBorins @bnoordhuis @nodejs-github-bot