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

test: refactor event-emitter-check-listener-leaks #13164

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented May 23, 2017

  • add block-scoping
  • use common.mustCall() on callbacks that should not execute
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test events

* add block-scoping
* use common.mustCall() on callbacks that should not execute
@Trott Trott added events Issues and PRs related to the events subsystem / EventEmitter. test Issues and PRs related to the tests. labels May 23, 2017
@refack
Copy link
Contributor

refack commented May 23, 2017

Since github does such a bad job of comparing this, here's an export from BeyondCompare4:
https://codepen.io/refack/full/XRoqBV/

@Trott
Copy link
Member Author

Trott commented May 23, 2017

@refack GitHub provides a much more usable diff if you pass w=1 as a URL parameter which tells it to suppress whitespace-only changes (in this case, indentation): https://github.com/nodejs/node/pull/13164/files?w=1

(And if you already knew that, consider it a notification for other readers who might be unaware.)

@refack
Copy link
Contributor

refack commented May 23, 2017

😲

@Trott
Copy link
Member Author

Trott commented May 24, 2017

@Trott
Copy link
Member Author

Trott commented May 25, 2017

Landed in 16357f2

@Trott Trott closed this May 25, 2017
Trott added a commit to Trott/io.js that referenced this pull request May 25, 2017
* add block-scoping
* use common.mustCall() on callbacks that should not execute

PR-URL: nodejs#13164
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
jasnell pushed a commit that referenced this pull request May 28, 2017
* add block-scoping
* use common.mustCall() on callbacks that should not execute

PR-URL: #13164
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell jasnell mentioned this pull request May 28, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@MylesBorins
Copy link
Contributor

This does not land cleanly in LTS. Please feel free to manually backport. Please also feel free to replace do-not-land if it is being backported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
events Issues and PRs related to the events subsystem / EventEmitter. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants