-
Notifications
You must be signed in to change notification settings - Fork 31k
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
watch: check parent and child path properly #57425
base: main
Are you sure you want to change the base?
Conversation
bb7f643
to
7a81702
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57425 +/- ##
==========================================
+ Coverage 90.21% 90.23% +0.01%
==========================================
Files 629 629
Lines 185126 184963 -163
Branches 36225 36223 -2
==========================================
- Hits 167016 166900 -116
+ Misses 11055 11019 -36
+ Partials 7055 7044 -11
🚀 New features to boost your workflow:
|
a9aa5b0
to
711c301
Compare
Hi folks @nodejs/build - could you take a look at this failure |
// The file is reloaded due to file watching | ||
if (completeCount === 2) { | ||
child.kill(); | ||
await once(child, 'exit'); |
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.
nit: I missed that earlier, but we don't really need to wait for the 'exit'
event here, since we already await for child.stdout
to close in the for await
. We can also keep it, it's fine
await once(child, 'exit'); |
if (line.startsWith('Completed running')) { | ||
completeCount++; | ||
} |
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.
btw should we skip running appendFileSync
and child.kill
if there are unrelated lines??
if (line.startsWith('Completed running')) { | |
completeCount++; | |
} | |
if (!line.startsWith('Completed running')) { | |
continue; | |
} | |
completeCount++; |
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.
Yeah I think that would be better to avoid unnecessary appends
co-authored-by: Jake Yuesong Li <[email protected]>
Fixes: #57422