Skip to content

Comments

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

Closed
Flarna wants to merge 1 commit intonodejs:v12.x-stagingfrom
dynatrace-oss-contrib:v12-thenables-v8
Closed

[v12.x backport] deps: V8: cherry-pick eec10a2fd8fa#34300
Flarna wants to merge 1 commit intonodejs:v12.x-stagingfrom
dynatrace-oss-contrib:v12-thenables-v8

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 •
edited by Flarna
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/32276/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3234/

@Flarna Flarna mentioned this pull request Jul 10, 2020
Closed
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@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

richardlau 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?

#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

codebytere commented Jul 12, 2020

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

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@Qard @Flarna
deps: V8: cherry-pick eec10a2fd8fa
04e4b4c 
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]>
@Flarna Flarna force-pushed the v12-thenables-v8 branch from b584efc to 04e4b4c Compare July 12, 2020 18:09
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 12, 2020

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

@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
@Qard @codebytere
deps: V8: cherry-pick eec10a2fd8fa
d1912e0 
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

codebytere commented Jul 13, 2020

Landed in d1912e0

@codebytere codebytere closed this Jul 13, 2020
@Flarna Flarna deleted the v12-thenables-v8 branch July 13, 2020 19:27
@github-actions github-actions bot mentioned this pull request Jul 14, 2020
Open
10 tasks
codebytere pushed a commit that referenced this pull request Jul 14, 2020
@Qard @codebytere
deps: V8: cherry-pick eec10a2fd8fa
c7c713a 
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]>
This was referenced Jul 15, 2020
Open
Open
Open
@Flarna Flarna restored the v12-thenables-v8 branch August 14, 2020 11:17
@Flarna Flarna mentioned this pull request Aug 14, 2020
Closed
@Flarna Flarna deleted the v12-thenables-v8 branch August 20, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@codebytere codebytere codebytere approved these changes

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Flarna @nodejs-github-bot @richardlau @codebytere @addaleax @Qard