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

doc: remove reference to resolved child_process v8 issue #51467

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

isker
Copy link
Contributor

@isker isker commented Jan 15, 2024

The linked v8 issue is closed. The NodeJS half of that issue has also been resolved. While there still may be issues related to child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138

The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Jan 15, 2024
@isker
Copy link
Contributor Author

isker commented Jan 15, 2024

I could remove the entire paragraph, as it was added all at once, but I figure the bit I've left is not wrong. Let me know if you think it should be removed.

@kvakil kvakil self-requested a review January 16, 2024 23:44
Copy link
Contributor

@kvakil kvakil left a comment

Choose a reason for hiding this comment

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

Thanks, I missed this line! Can you delete the paragraph entirely? It's obviously true that any asynchronous operation will "perform[] memory operations synchronously", the previous wording made sense as a way of emphasizing a particularly large cost which I think is no longer necessary to mention.

@isker
Copy link
Contributor Author

isker commented Jan 17, 2024

Done.

@kvakil kvakil added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 17, 2024
@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 17, 2024
@nodejs-github-bot nodejs-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 Jan 17, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/51467
✔  Done loading data for nodejs/node/pull/51467
----------------------------------- PR info ------------------------------------
Title      doc: remove reference to resolved child_process v8 issue (#51467)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     isker:patch-2 -> nodejs:main
Labels     child_process, doc, author ready
Commits    2
 - doc: remove reference to resolved child_process v8 issue
 - Update child_process.md
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/51467
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: https://github.com/nodejs/node/pull/48523
Refs: https://github.com/nodejs/performance/issues/138
Reviewed-By: Keyhan Vakil 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51467
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: https://github.com/nodejs/node/pull/48523
Refs: https://github.com/nodejs/performance/issues/138
Reviewed-By: Keyhan Vakil 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 15 Jan 2024 03:47:37 GMT
   ✔  Approvals: 2
   ✔  - Keyhan Vakil (@kvakil): https://github.com/nodejs/node/pull/51467#pullrequestreview-1826255104
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51467#pullrequestreview-1828022124
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 51467
From https://github.com/nodejs/node
 * branch                  refs/pull/51467/merge -> FETCH_HEAD
✔  Fetched commits as 6ab15a1aa8de..9cca93061688
--------------------------------------------------------------------------------
[main cfaf7262ca] doc: remove reference to resolved child_process v8 issue
 Author: Ian Kerins 
 Date: Sun Jan 14 22:46:09 2024 -0500
 1 file changed, 1 insertion(+), 3 deletions(-)
[main b51f83b9b7] Update child_process.md
 Author: Ian Kerins 
 Date: Wed Jan 17 01:44:46 2024 +0000
 1 file changed, 4 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)

Executing: git node land --amend --yes
⚠ Found Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381, skipping..
⚠ Found Refs: #48523, skipping..
⚠ Found Refs: nodejs/performance#138, skipping..
--------------------------------- New Message ----------------------------------
doc: remove reference to resolved child_process v8 issue

The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil kvakil@sylph.kvakil.me
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD 4afd586645] doc: remove reference to resolved child_process v8 issue
Author: Ian Kerins git@isk.haus
Date: Sun Jan 14 22:46:09 2024 -0500
1 file changed, 1 insertion(+), 3 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Update child_process.md

PR-URL: #51467
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
Reviewed-By: Keyhan Vakil kvakil@sylph.kvakil.me
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD c0a9578c16] Update child_process.md
Author: Ian Kerins git@isk.haus
Date: Wed Jan 17 01:44:46 2024 +0000
1 file changed, 4 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/7560845377

@debadree25 debadree25 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jan 18, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 18, 2024
@nodejs-github-bot nodejs-github-bot merged commit eb4432c into nodejs:main Jan 18, 2024
28 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in eb4432c

Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Jan 22, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 2, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Feb 15, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
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. child_process Issues and PRs related to the child_process subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants