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

[v12.x backport] deps: V8: cherry-pick eec10a2fd8fa #34300

Closed

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Jul 10, 2020

Original commit message:

[promisehook] Add before/after hooks to thenable tasks

This will allow Node.js to properly track async context in thenables.

Change-Id: If441423789a78307a57ad7e645daabf551cddb57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
Reviewed-by: Camillo Bruni <[email protected]>
Reviewed-by: Sathya Gunasekaran  <[email protected]>
Commit-Queue: Gus Caplan <[email protected]>
Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: Michaël Zasso [email protected]
Reviewed-By: Gus Caplan [email protected]
Reviewed-By: Gerhard Stöbich [email protected]
Reviewed-By: Michael Dawson [email protected]

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v12.x v8 engine Issues and PRs related to the V8 dependency. labels Jul 10, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 10, 2020

@nodejs-github-bot

This comment has been minimized.

@Flarna
Copy link
Member Author

Flarna commented Jul 11, 2020

I think the failed lint build is unrelated as v12.x-staging shows the same failure (https://ci.nodejs.org/job/node-test-linter/35727/)

Maybe related to backporting of #33852?

@richardlau
Copy link
Member

I think the failed lint build is unrelated as v12.x-staging shows the same failure (https://ci.nodejs.org/job/node-test-linter/35727/)

Maybe related to backporting of #33852?

#30607 updated tools/icu/README.md and removed code blocks from the file before #33852 landed and changed/tightened up the lint rules. #30607 is marked dont-land-on-v12.x so those code blocks still exist on 12.x.

Probably the simplest thing to do is fixup the code fences in 12.x prior to cherry-picking the commit from #33852. cc @codebytere

@codebytere
Copy link
Member

@Flarna fixed it up on staging - if you rebase this should be good to go

@nodejs-github-bot

This comment has been minimized.

Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Sathya Gunasekaran  <[email protected]>
    Commit-Queue: Gus Caplan <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: nodejs#33778
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

@Flarna
Copy link
Member Author

Flarna commented Jul 12, 2020

@codebytere Thanks. Rebased, CI is green now.

@Flarna Flarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 13, 2020
codebytere pushed a commit that referenced this pull request Jul 13, 2020
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Sathya Gunasekaran  <[email protected]>
    Commit-Queue: Gus Caplan <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Backport-PR-URL: #34300
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@codebytere
Copy link
Member

Landed in d1912e0

@codebytere codebytere closed this Jul 13, 2020
@Flarna Flarna deleted the v12-thenables-v8 branch July 13, 2020 19:27
codebytere pushed a commit that referenced this pull request Jul 14, 2020
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <[email protected]>
    Reviewed-by: Sathya Gunasekaran  <[email protected]>
    Commit-Queue: Gus Caplan <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

PR-URL: #33778
Backport-PR-URL: #34300
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@Flarna Flarna restored the v12-thenables-v8 branch August 14, 2020 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants