Skip to content

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

@thefourtheyethefourtheye added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Jun 29, 2016
@thefourtheyethefourtheyeforce-pushed the events-doc-improvment branch from 2730b17 to 5ed2bdcCompareJune 29, 2016 17:55
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

@mscdexmscdexJun 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

Great to see this. Left some comments!

@thefourtheyethefourtheyeforce-pushed the events-doc-improvment branch from 5ed2bdc to 7f51b34CompareJune 30, 2016 17:08
@thefourtheye
Copy link
ContributorAuthor

Addressed comments @jasnell and @mscdex

@jasnell
Copy link
Member

LGTM

@thefourtheye
Copy link
ContributorAuthor

Bump!

@thefourtheye
Copy link
ContributorAuthor

Landing this tomorrow if there are no more comments.

@thefourtheye
Copy link
ContributorAuthor

Landed in af49158. Thanks @jasnell :-)

@thefourtheyethefourtheye deleted the events-doc-improvment branch July 20, 2016 04:44
thefourtheye added a commit that referenced this pull request Jul 20, 2016
evanlucas pushed a commit that referenced this pull request Jul 21, 2016
@evanlucasevanlucas mentioned this pull request Jul 21, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.eventsIssues and PRs related to the events subsystem / EventEmitter.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@thefourtheye@jasnell@mscdex@Fishrock123@MylesBorins