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

debugger: bind to random port with --inspect=0 #8080

Closed
develar opened this issue Aug 12, 2016 · 10 comments
Closed

debugger: bind to random port with --inspect=0 #8080

develar opened this issue Aug 12, 2016 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. inspector Issues and PRs related to the V8 inspector protocol

Comments

@develar
Copy link

develar commented Aug 12, 2016

As #5025, but for --inspect. #5025 (comment)

@mscdex mscdex added debugger inspector Issues and PRs related to the V8 inspector protocol and removed debugger labels Aug 12, 2016
@ofrobots ofrobots added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Aug 31, 2016
@bmeck
Copy link
Member

bmeck commented Sep 2, 2016

I am guessing you would also want/need the url to load up in devtools?

@Trott Trott added feature request Issues that request new features to be added to Node.js. semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-minor PRs that contain new features and should be released in the next minor version. labels Sep 2, 2016
@after-ephemera
Copy link

I'd love to take a shot at helping on this. Can someone direct me where to start?

@cjihrig
Copy link
Contributor

cjihrig commented May 12, 2017

@azjkjensen this is probably a good place to start. Allowing 0 here seems to make it work for me, but I didn't run the full test suite, and there could be other nuances.

@after-ephemera
Copy link

@cjihrig are you suggesting that I just add a check for if result == 0 or do I need to add the random port logic as well?

@cjihrig
Copy link
Contributor

cjihrig commented May 16, 2017

@azjkjensen I was suggesting allowing result == 0.

@after-ephemera
Copy link

@cjihrig it looks like that change works, I just need to write a test for it and then I'll make a PR. Is a single test usually sufficient for a change like this? This is my first time contributing so such a large project.

@refack
Copy link
Contributor

refack commented May 16, 2017

Is a single test usually sufficient for a change like this?

Use your judgment, if you can think of more than one relevant situation write more than one test...
Good place to look is at test that already check --inspect or --inspect-brk like test/parallel/test-debug-brk.js

@refack
Copy link
Contributor

refack commented May 16, 2017

P.S. if #13002 lands, you should add a line there too.
P.S. #13002 landed, you should add a line there too.

@after-ephemera
Copy link

@refack looks like someone beat me to the punch in #5025. I think this issue is good to close given that the --inspect=0 has now been pushed in v8.1.0.

@refack
Copy link
Contributor

refack commented Jun 8, 2017

@azjkjensen you snooze you lose (or win depends on your POV) 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

No branches or pull requests

8 participants