Skip to content

stream: use structuredClone instead of v8 - #45611

Merged
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
anonrig:stream/structured-clone
Nov 27, 2022
Merged

stream: use structuredClone instead of v8#45611
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
anonrig:stream/structured-clone

Conversation

@anonrig

@anonriganonrig commented Nov 24, 2022

Copy link
Copy Markdown
Member

This pull request replaces deserialize(serialize(value)) with structuredClone(value) due to performance.

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Nov 24, 2022
@anonriganonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 24, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 24, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadlib/internal/webstreams/readablestream.js Outdated
@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 24, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 24, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 25, 2022
@tniessen

Copy link
Copy Markdown
Member

It would be cool not to have empty commit messages and empty PR descriptions. What is being done or how it is being done is often less relevant than the reasoning behind it.

Comment threadlib/internal/webstreams/readablestream.js Outdated
@anonrig

Copy link
Copy Markdown
MemberAuthor

It would be cool not to have empty commit messages and empty PR descriptions. What is being done or how it is being done is often less relevant than the reasoning behind it.

Thanks for the feedback. I'll add descriptions from now on. I've also updated the description for this pull request.

@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 26, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/45611
✔ Done loading data for nodejs/node/pull/45611
----------------------------------- PR info ------------------------------------
Title stream: use structuredClone instead of v8 (#45611)
Author Yagiz Nizipli (@anonrig)
Branch anonrig:stream/structured-clone -> nodejs:main
Labels author ready, needs-ci, dont-land-on-v14.x, dont-land-on-v16.x, web streams
Commits 3
- stream: use structuredClone instead of v8
- fixup! stream: use structuredClone instead of v8
- fixup! stream: use structuredClone instead of v8
Committers 1
- Yagiz Nizipli PR-URL: https://github.com/nodejs/node/pull/45611
Reviewed-By: Erick Wendel Reviewed-By: Antoine du Hamel Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45611
Reviewed-By: Erick Wendel Reviewed-By: Antoine du Hamel Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
ℹ This PR was created on Thu, 24 Nov 2022 16:56:34 GMT
✔ Approvals: 3
✔ - Erick Wendel (@erickwendel): https://github.com/nodejs/node/pull/45611#pullrequestreview-1193461926
✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/45611#pullrequestreview-1193532434
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/45611#pullrequestreview-1193580550
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2022-11-24T18:52:43Z: https://ci.nodejs.org/job/node-test-pull-request/48167/
⚠ Commits were pushed after the last Full PR CI run:
⚠ - fixup! stream: use structuredClone instead of v8
- Querying data for job/node-test-pull-request/48167/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3554598277

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonriganonrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 27, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 27, 2022
@nodejs-github-bot
nodejs-github-bot merged commit dda7a21 into nodejs:mainNov 27, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in dda7a21

targos pushed a commit that referenced this pull request Dec 12, 2022
PR-URL: #45611
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targostargos mentioned this pull request Dec 12, 2022
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45611
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45611
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
PR-URL: #45611
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.needs-ciPRs that need a full CI run.web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@anonrig@nodejs-github-bot@tniessen@sindresorhus@lpinca@ErickWendel@aduh95