Skip to content

doc: general improvements to events documentation - #7480

Closed
thefourtheye wants to merge 2 commits into
nodejs:masterfrom
thefourtheye:events-doc-improvment
Closed

doc: general improvements to events documentation#7480
thefourtheye wants to merge 2 commits into
nodejs:masterfrom
thefourtheye:events-doc-improvment

Conversation

@thefourtheye

Copy link
Copy Markdown
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
@thefourtheye
thefourtheyeforce-pushed the events-doc-improvment branch from 2730b17 to 5ed2bdcCompareJune 29, 2016 17:55
Comment threaddoc/api/events.md Outdated

Copy link
Copy Markdown
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
Copy Markdown
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...

@mscdexmscdexJun 30, 2016

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Member

Great to see this. Left some comments!

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

Copy link
Copy Markdown
ContributorAuthor

Addressed comments @jasnell and @mscdex

@jasnell

Copy link
Copy Markdown
Member

LGTM

@thefourtheye

Copy link
Copy Markdown
ContributorAuthor

Bump!

@thefourtheye

Copy link
Copy Markdown
ContributorAuthor

Landing this tomorrow if there are no more comments.

@thefourtheye

Copy link
Copy Markdown
ContributorAuthor

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