-
Notifications
You must be signed in to change notification settings - Fork 113
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
fix(v8): fix spawning of subprocesses #726
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #726 +/- ##
=======================================
Coverage 83.38% 83.38%
=======================================
Files 37 37
Lines 4158 4158
=======================================
Hits 3467 3467
Misses 691 691 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, maybe we should add tests for these flows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not quite right:
- Output from git command appears in the terminal
- Error is still here
$ node /Users/mzasso/git/nodejs/node-core-utils/bin/git-node.js v8 --v8-dir ~/git/chromium/v8/v8 major
❯ Update local V8 clone
⠸ Fetch V8
❯ Update local V8 clone
❯ Update local V8 clone
⠴ Fetch V8
◼ Major V8 update
◼ Update V8 version in maintaining-dependencies.md
◼ Commit V8 update
◼ Update version numbers
From https://chromium.googlesource.com/v8/v8
* [new branch] 11.9.50 -> origin/11.9.50
* [new branch] 11.9.51 -> origin/11.9.51
+ bd2ab654247...eb941aa621b canary -> origin/canary (forced update)
4f2f8661fa4..5513eb17d41 lkgr -> origin/lkgr
85d36de7fb4..36d5e54d760 main -> origin/main
+ bd2ab654247...eb941aa621b roll -> origin/roll (forced update)
✔ Update local V8 clone
❯ Major V8 update
✔ Update local V8 clone
❯ Major V8 update
✔ Get current V8 version
⠙ Checkout V8 branch
◼ Remove deps/v8
◼ Clone branch to deps/v8
◼ Remove deps/v8/.git
◼ Track all files in deps/v8
◼ Update V8 DEPS
◼ Apply Node-specific changes
◼ Update V8 version in maintaining-dependencies.md
◼ Commit V8 update
◼ Update version numbers
error: branch '11.9.51' not found
Certainly, but that's not easy to do (and V8 repo takes a very long time to clone). |
It's better (doesn't fail anymore), but the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with my last commit. Git output is no longer intertwined with Listr
Refs: #722 (comment)
Fixes: #723