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.

Sign up for GitHub

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

Jump to bottom

async_hooks: make sure .{en|dis}able() === this #13418

Closed
wants to merge 1 commit into from
Closed

async_hooks: make sure .{en|dis}able() === this #13418

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Jun 2, 2017

Make sure that hook.enable() and hook.disable() return hook
consistently, as the documentation indicates.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

async_hooks /cc @nodejs/async_hooks

@addaleax
async_hooks: make sure .{en|dis}able() === this
980b7b9 
Make sure that `hook.enable()` and `hook.disable()` return `hook`
consistently, as the documentation indicates.
@nodejs-github-bot nodejs-github-bot added the async_hooks Issues and PRs related to the async hooks subsystem. label Jun 2, 2017
trevnorris
trevnorris approved these changes Jun 6, 2017
View reviewed changes
Copy link
Contributor

@trevnorris trevnorris left a comment

Choose a reason for hiding this comment

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

Oops. Oversight on my part. Thanks for the fix + test.

@addaleax
Copy link
Member Author

addaleax commented Jun 7, 2017

CI: https://ci.nodejs.org/job/node-test-commit/10428/

@addaleax
Copy link
Member Author

addaleax commented Jun 8, 2017

Landed in 6318078

@addaleax addaleax closed this Jun 8, 2017
@addaleax addaleax deleted the async-hooks-enable-return-this branch June 8, 2017 18:21
addaleax added a commit that referenced this pull request Jun 8, 2017
@addaleax
async_hooks: make sure .{en|dis}able() === this
6318078 
Make sure that `hook.enable()` and `hook.disable()` return `hook`
consistently, as the documentation indicates.

PR-URL: #13418
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
addaleax added a commit that referenced this pull request Jun 10, 2017
@addaleax
async_hooks: make sure .{en|dis}able() === this
bf61d97 
Make sure that `hook.enable()` and `hook.disable()` return `hook`
consistently, as the documentation indicates.

PR-URL: #13418
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
@addaleax addaleax mentioned this pull request Jun 10, 2017
Merged
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@trevnorris trevnorris trevnorris approved these changes

@jasnell jasnell jasnell approved these changes

@AndreasMadsen AndreasMadsen AndreasMadsen approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
async_hooks Issues and PRs related to the async hooks subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@addaleax @refack @trevnorris @jasnell @AndreasMadsen @cjihrig @MylesBorins @nodejs-github-bot