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

async_hooks: consistent internal naming #15569

Closed
wants to merge 2 commits into from
Closed

async_hooks: consistent internal naming #15569

wants to merge 2 commits into from

Conversation

AndreasMadsen
Copy link
Member

@AndreasMadsen AndreasMadsen commented Sep 23, 2017

Our internal naming and API doesn't match the external API. This fixes that.

Note: we actually have a bit of external API with the old naming scheme, promiseHandle.parentId, newUid and initTriggerId. I didn't touch those, as I wanted to just focus on the internal naming.

refack: moved description to top

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

async_hooks

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 23, 2017
@AndreasMadsen
Copy link
Member Author

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

/cc @nodejs/async_hooks

@AndreasMadsen AndreasMadsen added async_hooks Issues and PRs related to the async hooks subsystem. async_wrap labels Sep 23, 2017
@AndreasMadsen
Copy link
Member Author

AndreasMadsen commented Sep 23, 2017

@AndreasMadsen
Copy link
Member Author

Because of the potential for merge conflicts, I will merge this as soon as 72 hours have passed. If anyone have objections, now is the time to say so.

@AndreasMadsen
Copy link
Member Author

@jasnell
Copy link
Member

jasnell commented Sep 25, 2017

Partial re-run of CI: https://ci.nodejs.org/job/node-test-commit-plinux/11672/

@AndreasMadsen
Copy link
Member Author

AndreasMadsen commented Sep 25, 2017

@jasnell Thanks, but it failed again. I think the CI is having some tough days.

New partial CI: https://ci.nodejs.org/job/node-test-commit-plinux/11686/

@AndreasMadsen
Copy link
Member Author

Landed in 3a69ef5

AndreasMadsen added a commit that referenced this pull request Sep 26, 2017
PR-URL: #15569
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 28, 2017
PR-URL: #15569
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 29, 2017
PR-URL: #15569
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 30, 2017
Original-PR-URL: nodejs/node#15569
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 30, 2017
PR-URL: ayojs#87
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-PR-URL: nodejs/node#15569
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 3, 2017
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
PR-URL: #15569
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

Opted for dont-land. LMK if you want to do a larger effort at backporting the current async_hooks implementation to v6.x

@AndreasMadsen
Copy link
Member Author

I'm always forgetting those don't land labels :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants