Fix #7065: cli help documentation for --inspect by nojvek · Pull Request #11660 · nodejs/node

nojvek · 2017-03-02T18:35:12Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

documentation

nojvek · 2017-03-02T18:36:48Z

Context in: #7086

nojvek · 2017-03-02T18:59:30Z

@joshgav @jasnell

sam-github · 2017-03-02T19:33:32Z

needs changing in https://github.com/nodejs/node/blob/master/doc/node.1, too

nojvek · 2017-03-02T22:53:38Z

is node.1 for man pages?

sam-github · 2017-03-02T23:45:13Z

Yes.

nojvek · 2017-03-03T18:03:06Z

Fixed.

fhinkel · 2017-03-09T10:21:37Z

Do reference-style links work if they are linked in another file?
[Chrome Debugging Protocol]: https://chromedevtools.github.io/debugger-protocol-viewer/ is defined in debugger.md.

small, nit... this could be reworded as:
V8 Inspector integration uses the [Chrome Debugging Protocol]{} to attach Node.js instances to debugging and profile tools such as Chrome DevTools and various IDEs.

nojvek · 2017-03-09T14:04:37Z

That's how the inspector debugger.md documentation was done so I copied from there. I should point it to the correct link though. How do I build the documentation. Website to test this out?

…

On Thu, Mar 9, 2017 at 2:22 AM F. Hinkelmann ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In doc/api/cli.md <#11660 (comment)>: > @@ -94,6 +94,25 @@ Follows `require()`'s module resolution rules. `module` may be either a path to a file, or a node module name. +### `--inspect[=host:port]` + + +Activate inspector on host:port. Default is 127.0.0.1:9229. + +V8 Inspector integration allows attaching Chrome DevTools and IDEs to Node.js +instances for debugging and profiling. It uses the [Chrome Debugging Protocol][]. Do reference-style links work if they are linked in another file? [Chrome Debugging Protocol]: https://chromedevtools.github.io/debugger-protocol-viewer/ is defined in debugger.md. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVGVKhJt_6ywbPWy1hQBrPGBGXqwNks5rj9LVgaJpZM4MRXMz> .

fhinkel · 2017-03-09T16:57:38Z

make doc generates the docs, I think the html files end up in out/doc/api/. Do you want to check if we need to define
[Chrome Debugging Protocol]: https://chromedevtools.github.io/debugger-protocol-viewer/
in the file where you're using [Chrome Debugging Protocol]?

nojvek · 2017-03-09T19:02:49Z

Cli.md I believe debugger.md uses it too.

…

On Thu, Mar 9, 2017 at 8:58 AM F. Hinkelmann ***@***.***> wrote: make doc generates the docs, I think the html files end up in out/doc/api/. Do you want to check if we need to define [Chrome Debugging Protocol]: https://chromedevtools.github.io/debugger-protocol-viewer/ in the file where you're using [Chrome Debugging Protocol]? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVDnSUeIYOFvxqjlMA49lHde_wgq-ks5rkC-ngaJpZM4MRXMz> .

fhinkel · 2017-03-09T20:02:12Z

Sorry if I wasn't clear. debugger.md uses and defines a link to the debugging protocol. If we're using a reference-style link, we need to define it in the file where we use it. We can just copy this definition to the end of cli.md.

Could you also fix-up your commit messages according to this guide please.

nojvek · 2017-03-09T23:15:47Z

Perfect. I will make it happen

…

On Thu, Mar 9, 2017 at 12:02 PM F. Hinkelmann ***@***.***> wrote: Sorry if I wasn't clear. debugger.md uses and defines a link to the debugging protocol. If we're using a reference-style link, we need to define it in the file where we use it. We can just copy this <https://github.com/nodejs/node/blame/master/doc/api/debugger.md#L215> definition to the end of cli.md. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVHYiaF0d7inu-qYprD1VkARjubsHks5rkFrogaJpZM4MRXMz> .

cjihrig · 2017-03-13T14:39:23Z

I think --inspect was initially added in 6.3.0. It might have changed slightly since then, but luckily we have per item changelogs now.

cjihrig · 2017-03-13T14:44:34Z

I think this was introduced in 7.6.0.

nojvek · 2017-03-13T17:50:11Z

Thanks, I'll update it.

…

On Mon, Mar 13, 2017 at 7:45 AM, Colin Ihrig ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In doc/api/cli.md <#11660 (comment)>: > @@ -94,6 +94,25 @@ Follows `require()`'s module resolution rules. `module` may be either a path to a file, or a node module name. +### `--inspect[=host:port]` + + +Activate inspector on host:port. Default is 127.0.0.1:9229. + +V8 Inspector integration allows attaching Chrome DevTools and IDEs to Node.js +instances for debugging and profiling. It uses the [Chrome Debugging Protocol][]. + + +### `--inspect-brk[=host:port]` +<!-- YAML +added: v6.5.0 I think this was introduced in 7.6.0 <https://nodejs.org/en/blog/release/v7.6.0/>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVK1F6s6zv1MD4Y6E_7CSFW2TOr3Wks5rlVaHgaJpZM4MRXMz> .

nojvek · 2017-03-14T16:49:32Z

@cjihrig Thanks for the version number verification. I wasn't quite 100% sure how to do that.

I have updated the PR with your changes. Looks great, the links work. Would love to see this merged.

nojvek · 2017-03-14T17:02:30Z

Thanks Colin. I'm wondering who has to finally make a decision to make the merge? I first created a another PR regarding this almost an year ago. It'd make me dysphoric if it turned into another zombie PR.

…

On Tue, Mar 14, 2017 at 9:58 AM, Colin Ihrig ***@***.***> wrote: ***@***.**** approved this pull request. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVAmbJwVSZl2m6jOlDYEUpCvI606Vks5rlschgaJpZM4MRXMz> .

jasnell · 2017-03-14T17:29:22Z

small, nit... this could be reworded as:
V8 Inspector integration uses the [Chrome Debugging Protocol]{} to attach Node.js instances to debugging and profile tools such as Chrome DevTools and various IDEs.

cjihrig · 2017-03-14T17:33:39Z

@nojvek any committer can land the PR. It should be good after @jasnell's nit is addressed. @fhinkel are you ok with it?

nojvek · 2017-03-14T17:51:22Z

@jasnell that actually seems confusing. It seems like Node.js is attaching to devtools but its other way around. How about this? ``` V8 inspector integration allows tools such as Chrome DevTools and IDEs to debug and profile Node.js instances. The tools attach to Node.js instances on a tcp debug port and communicate using the [Chrome Debugging Protocol][] ```

…

On Tue, Mar 14, 2017 at 10:30 AM, James M Snell ***@***.***> wrote: ***@***.**** approved this pull request. ------------------------------ In doc/api/cli.md <#11660 (comment)>: > @@ -94,6 +94,25 @@ Follows `require()`'s module resolution rules. `module` may be either a path to a file, or a node module name. +### `--inspect[=host:port]` + + +Activate inspector on host:port. Default is 127.0.0.1:9229. + +V8 Inspector integration allows attaching Chrome DevTools and IDEs to Node.js +instances for debugging and profiling. It uses the [Chrome Debugging Protocol][]. small, nit... this could be reworded as: V8 Inspector integration uses the [Chrome Debugging Protocol]{} to attach Node.js instances to debugging and profile tools such as Chrome DevTools and various IDEs. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVAfza5UFh_1lc0c0YEPu_1m-jqDLks5rls62gaJpZM4MRXMz> .

jasnell · 2017-03-15T05:16:30Z

small nit here... need to line wrap a max 80 chars

nojvek · 2017-03-15T17:18:12Z

will do.

…

On Tue, Mar 14, 2017 at 10:17 PM, James M Snell ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In doc/api/cli.md <#11660 (comment)>: > @@ -94,6 +94,23 @@ Follows `require()`'s module resolution rules. `module` may be either a path to a file, or a node module name. +### `--inspect[=host:port]` + + +Activate inspector on host:port. Default is 127.0.0.1:9229. + +V8 inspector integration allows tools such as Chrome DevTools and IDEs to debug and profile Node.js instances. The tools attach to Node.js instances via a tcp port and communicate using the [Chrome Debugging Protocol][]. small nit here... need to line wrap a max 80 chars — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVPmBucxCLgWpfLV7S9KvkgULCqVcks5rl3RQgaJpZM4MRXMz> .

Adding documentation to node.1 and cli.md

nojvek · 2017-03-16T02:36:29Z

Done. Let me know if you need anything else.

…

On Wed, Mar 15, 2017 at 10:17 AM, Noj Vek ***@***.***> wrote: will do. On Tue, Mar 14, 2017 at 10:17 PM, James M Snell ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In doc/api/cli.md > <#11660 (comment)>: > > > @@ -94,6 +94,23 @@ Follows `require()`'s module resolution > rules. `module` may be either a path to a file, or a node module name. > > > +### `--inspect[=host:port]` > + > + > +Activate inspector on host:port. Default is 127.0.0.1:9229. > + > +V8 inspector integration allows tools such as Chrome DevTools and IDEs to debug and profile Node.js instances. The tools attach to Node.js instances via a tcp port and communicate using the [Chrome Debugging Protocol][]. > > small nit here... need to line wrap a max 80 chars > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#11660 (review)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AA-JVPmBucxCLgWpfLV7S9KvkgULCqVcks5rl3RQgaJpZM4MRXMz> > . >

Adding documentation to node.1 and cli.md PR-URL: #11660 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell · 2017-03-17T17:37:25Z

Landed in 38299d2

nojvek · 2017-03-17T17:39:31Z

wohooo! 😁🎉🎉🎇🎇〽〽

…

On Fri, Mar 17, 2017 at 10:26 AM, James M Snell ***@***.***> wrote: ***@***.**** approved this pull request. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11660 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-JVAw3QHztynnUl91i_Wt4nbo-GDC7ks5rmsJJgaJpZM4MRXMz> .

joshgav · 2017-03-17T17:55:08Z

Thanks @nojvek!

Adding documentation to node.1 and cli.md PR-URL: nodejs#11660 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations. labels Mar 2, 2017

nojvek mentioned this pull request Mar 2, 2017
Fixes #7065: cli help documentation for --debug and --debug-brk #7086
Closed
4 tasks

eugeneo approved these changes Mar 2, 2017
View reviewed changes

fhinkel reviewed Mar 9, 2017
View reviewed changes

cjihrig requested changes Mar 13, 2017
View reviewed changes

cjihrig approved these changes Mar 14, 2017
View reviewed changes

jasnell approved these changes Mar 14, 2017
View reviewed changes

jasnell reviewed Mar 15, 2017
View reviewed changes

Comment threaddoc/api/cli.md Outdated
jasnellMar 15, 2017
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small nit here... need to line wrap a max 80 chars

Fix#7065: cli help documentation for --inspect
84e11c1
Adding documentation to node.1 and cli.md

jasnell approved these changes Mar 17, 2017
View reviewed changes

jasnell closed this Mar 17, 2017

nojvek deleted the cli-params-doc branch March 17, 2017 17:51

italoacasas mentioned this pull request Mar 21, 2017
v7.7.4 Release Proposal #11941
Merged

MylesBorins added dont-land-on-v4.x labels Apr 18, 2017

Uh oh!

Uh oh!

Conversation

nojvek commented Mar 2, 2017

Checklist

Affected core subsystem(s)

Uh oh!

nojvek commented Mar 2, 2017

Uh oh!

nojvek commented Mar 2, 2017

Uh oh!

sam-github commented Mar 2, 2017

Uh oh!

nojvek commented Mar 2, 2017

Uh oh!

sam-github commented Mar 2, 2017 via email

Uh oh!

nojvek commented Mar 3, 2017

Uh oh!

fhinkelMar 9, 2017

Choose a reason for hiding this comment

Uh oh!

jasnellMar 14, 2017

Choose a reason for hiding this comment

Uh oh!

nojvek commented Mar 9, 2017 via email

Uh oh!

fhinkel commented Mar 9, 2017

Uh oh!

nojvek commented Mar 9, 2017 via email

Uh oh!

fhinkel commented Mar 9, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nojvek commented Mar 9, 2017 via email

Uh oh!

cjihrigMar 13, 2017

Choose a reason for hiding this comment

Uh oh!

cjihrigMar 13, 2017

Choose a reason for hiding this comment

Uh oh!

nojvek commented Mar 13, 2017 via email

Uh oh!

nojvek commented Mar 14, 2017

Uh oh!

nojvek commented Mar 14, 2017 via email

Uh oh!

jasnellMar 14, 2017

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Mar 14, 2017

Uh oh!

nojvek commented Mar 14, 2017 via email

Uh oh!

jasnellMar 15, 2017

Choose a reason for hiding this comment

Uh oh!

nojvek commented Mar 15, 2017 via email

Uh oh!

nojvek commented Mar 16, 2017 via email

Uh oh!

jasnell commented Mar 17, 2017

Uh oh!

nojvek commented Mar 17, 2017 via email

Uh oh!

joshgav commented Mar 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

fhinkel commented Mar 9, 2017•
edited
Loading