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

doc: general improvements to events documentation #7480

Closed

Conversation

thefourtheye
Copy link
Contributor

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, events

Description of change

general doc improvements

cc @nodejs/documentation

@thefourtheye thefourtheye added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Jun 29, 2016
@thefourtheye thefourtheye force-pushed the events-doc-improvment branch from 2730b17 to 5ed2bdc Compare June 29, 2016 17:55
[`domain`][] module (_Note, however, that the `domain` module has been
deprecated_).
To guard against crashing the Node.js process, a listener can be registered
for the `process.on('uncaughtException')` event or the [`domain`][] module can be
Copy link
Contributor

Choose a reason for hiding this comment

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

"the process.on('uncaughtException') event" is confusing since it's a piece of code and not just the event name.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps:

To guard against crashing the Node.js process, a listener can be registered on
the `process` object's `'uncaughtException'` event
(e.g. `process.on('uncaughtException', (err) => /* ... */)`) or the [`domain`][] module
can be used...

Copy link
Contributor

@mscdex mscdex Jun 30, 2016

Choose a reason for hiding this comment

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

That seems mostly alright to me, except I think if we're going to have a code example like that, it shouldn't be inline, it takes up too much space IMHO.

@jasnell
Copy link
Member

jasnell commented Jun 29, 2016

Great to see this. Left some comments!

@thefourtheye thefourtheye force-pushed the events-doc-improvment branch from 5ed2bdc to 7f51b34 Compare June 30, 2016 17:08
@thefourtheye
Copy link
Contributor Author

Addressed comments @jasnell and @mscdex

@jasnell
Copy link
Member

jasnell commented Jun 30, 2016

LGTM

@thefourtheye
Copy link
Contributor Author

Bump!

@thefourtheye
Copy link
Contributor Author

Landing this tomorrow if there are no more comments.

@thefourtheye
Copy link
Contributor Author

Landed in af49158. Thanks @jasnell :-)

@thefourtheye thefourtheye deleted the events-doc-improvment branch July 20, 2016 04:44
thefourtheye added a commit that referenced this pull request Jul 20, 2016
PR-URL: #7480
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request Jul 21, 2016
PR-URL: #7480
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants