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

lib,src,doc: remove usage of events.EventEmitter #2921

Closed

Conversation

thefourtheye
Copy link
Contributor

The events module already exports EventEmitter constructor function
So, we don't have to use events.EventEmitter to access it.

Refer: #2896

cc @silverwind @evanlucas

The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: nodejs#2896
@thefourtheye thefourtheye added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. test Issues and PRs related to the tests. labels Sep 16, 2015
@@ -117,7 +117,7 @@ function onServerResponseClose() {
// array. That is, in the example below, b still gets called even though
// it's been removed by a:
//
// var obj = new events.EventEmitter;
// var obj = new events();
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe var obj = new EventEmitter();?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evanlucas I updated this. Thanks :-)

The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: nodejs#2896
@thefourtheye thefourtheye force-pushed the events-eventemitter-usage branch 2 times, most recently from 742a9ad to 705ff31 Compare September 19, 2015 06:47
@thefourtheye
Copy link
Contributor Author

Removing the test changes, after a lengthy discussion with @brendanashworth ;-)

@thefourtheye thefourtheye removed the test Issues and PRs related to the tests. label Sep 19, 2015
@thefourtheye thefourtheye changed the title lib,src,test,doc: remove usage of events.EventEmitter lib,src,doc: remove usage of events.EventEmitter Sep 19, 2015
@silverwind
Copy link
Contributor

@silverwind
Copy link
Contributor

LGTM

1 similar comment
@targos
Copy link
Member

targos commented Sep 22, 2015

LGTM

thefourtheye added a commit that referenced this pull request Sep 22, 2015
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: #2896

PR-URL: #2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
thefourtheye added a commit that referenced this pull request Sep 22, 2015
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: #2896

PR-URL: #2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@thefourtheye
Copy link
Contributor Author

Thanks for the review, landed in f32a606 and 7953c83

@thefourtheye thefourtheye deleted the events-eventemitter-usage branch September 22, 2015 18:54
thefourtheye added a commit that referenced this pull request Sep 22, 2015
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: #2896

PR-URL: #2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
thefourtheye added a commit that referenced this pull request Sep 22, 2015
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.

Refer: #2896

PR-URL: #2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@rvagg rvagg mentioned this pull request Sep 22, 2015
@targos targos mentioned this pull request Oct 24, 2015
stojanovic added a commit to stojanovic/koa that referenced this pull request Oct 24, 2015
The 'events' module already exports 'EventEmitter' constructor function - nodejs/node#2921
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.

None yet

4 participants