Skip to content

node: deprecate process.EventEmitter - #5049

Merged
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:depee
Feb 4, 2016
Merged

node: deprecate process.EventEmitter#5049
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:depee

Conversation

@evanlucas

Copy link
Copy Markdown
Contributor

The comment stating it was deprecated was added in 2011 via
4ef8f06. It is time to actually deprecate it.

@mscdexmscdex added the process Issues and PRs related to the process subsystem. label Feb 3, 2016
@mscdex

Copy link
Copy Markdown
Contributor

/cc @ChALkeR

Does anyone use this anywhere?

@evanlucas

Copy link
Copy Markdown
ContributorAuthor

FWIW here is a github search for it as well. https://github.com/search?p=2&q=process.EventEmitter&type=Code&utf8=%E2%9C%93

@jasnell

Copy link
Copy Markdown
Member

LGTM

1 similar comment
@bnoordhuis

Copy link
Copy Markdown
Member

LGTM

@evanlucas

Copy link
Copy Markdown
ContributorAuthor

CI: https://ci.nodejs.org/job/node-test-pull-request/1524/

Was the decision that we could deprecate in a minor version, or did we go with major?

@ChALkeR

Copy link
Copy Markdown
Member

@mscdex This is used, though the usage is not high. Will post a list a bit later today.

@cjihrig

Copy link
Copy Markdown
Contributor

LGTM

1 similar comment
@thefourtheye

Copy link
Copy Markdown
Contributor

LGTM

@silverwind

Copy link
Copy Markdown
Contributor

@evanlucas you can get better results by quoting.

@evanlucas

Copy link
Copy Markdown
ContributorAuthor

oh nice

@ChALkeR

Copy link
Copy Markdown
Member

https://gist.github.com/ChALkeR/232fdf64be9c34c9c733, sorted by downloads count per month.
The first entry (array-difference) looks like a false positive to me.

I'm +1 for deprecating this, but in a semver-major, of course.

@ChALkeRChALkeR added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 4, 2016
@evanlucas

Copy link
Copy Markdown
ContributorAuthor

Major works for me

@Fishrock123Fishrock123 added this to the 6.0.0 milestone Feb 4, 2016
@silverwind

Copy link
Copy Markdown
Contributor

LGTM, looks to be low-impact.

@evanlucas

Copy link
Copy Markdown
ContributorAuthor

CI https://ci.nodejs.org/job/node-test-pull-request/1550/ one more time before landing since the last one was red.

The comment stating it was deprecated was added in 2011 via
4ef8f06. It is time to
actually deprecate it.
PR-URL: nodejs#5049
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
@evanlucas
evanlucas deleted the depee branch February 4, 2016 19:45
@evanlucas
evanlucas merged commit 25751be into nodejs:masterFeb 4, 2016
@evanlucas

Copy link
Copy Markdown
ContributorAuthor

Landed in 25751be. Thanks

@CangitCangit mentioned this pull request Feb 11, 2016
@jasnelljasnell mentioned this pull request Mar 17, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
The comment stating it was deprecated was added in 2011 via
4ef8f06. It is time to
actually deprecate it.
PR-URL: nodejs#5049
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
cjihrig added a commit that referenced this pull request May 24, 2016
process.EventEmitter was deprecated for v6. This commit removes
it for v7.
Refs: #5049
PR-URL: #6862
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

processIssues and PRs related to the process subsystem.semver-majorPRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@evanlucas@mscdex@jasnell@bnoordhuis@ChALkeR@cjihrig@thefourtheye@silverwind@Fishrock123