Skip to content

doc: improvements to events.markdown copy - #4468

Closed
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:doc-events-improvements
Closed

doc: improvements to events.markdown copy#4468
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:doc-events-improvements

Conversation

@jasnell

Copy link
Copy Markdown
Member

General improvements to events.markdown copy including a
bit of restructuring and improved examples

@jasnelljasnell added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Dec 29, 2015
@MylesBorins

Copy link
Copy Markdown
Contributor

LGTM

My only comment is that it seems like the first example might have too much inheritance logic that the basic emitter logic could be lost on a newer developer. Perhaps the first example should simply show setting up an instance of emitter?

Comment threaddoc/api/events.markdown Outdated

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.

immediately

@Qard

Qard commented Dec 30, 2015

Copy link
Copy Markdown
Member

One minor nit, but LGTM other than that.

@jasnell

Copy link
Copy Markdown
MemberAuthor

@thealphanerd ... given that inheriting from EventEmitter is the most common use case, I'd rather show that right up front

General improvements to events.markdown copy including a
bit of restructuring and improved examples
@jasnell
jasnellforce-pushed the doc-events-improvements branch from 203d1cd to abdc464CompareDecember 30, 2015 18:46
@jasnell

Copy link
Copy Markdown
MemberAuthor

@Qard ... nit addressed.

@Qard

Qard commented Dec 30, 2015

Copy link
Copy Markdown
Member

LGTM

@MylesBorins

Copy link
Copy Markdown
Contributor

@jasnell that is reasonable enough, and how it was done before. LGTM

jasnell added a commit that referenced this pull request Dec 30, 2015
General improvements to events.markdown copy including a
bit of restructuring and improved examples
PR-URL: #4468
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
@jasnell

Copy link
Copy Markdown
MemberAuthor

Landed in a0130e1

@jasnelljasnell closed this Dec 30, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
General improvements to events.markdown copy including a
bit of restructuring and improved examples
PR-URL: nodejs#4468
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
MylesBorins pushed a commit that referenced this pull request Jan 13, 2016
General improvements to events.markdown copy including a
bit of restructuring and improved examples
PR-URL: #4468
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
General improvements to events.markdown copy including a
bit of restructuring and improved examples
PR-URL: #4468
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
@MylesBorinsMylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
General improvements to events.markdown copy including a
bit of restructuring and improved examples
PR-URL: nodejs#4468
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
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.

3 participants

@jasnell@MylesBorins@Qard