-
Notifications
You must be signed in to change notification settings - Fork 572
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
feat: Pool size & running #535
Conversation
Tests needs fixing |
@dnlup Feel free to sort out the tests here if you want. But I was referring to the failures on master. |
bae5955
to
a7cf29a
Compare
a7cf29a
to
25856c3
Compare
Codecov Report
@@ Coverage Diff @@
## master #535 +/- ##
==========================================
- Coverage 99.63% 99.56% -0.07%
==========================================
Files 16 16
Lines 1366 1385 +19
==========================================
+ Hits 1361 1379 +18
- Misses 5 6 +1
Continue to review full report at Codecov.
|
719cf03
to
eed6638
Compare
f1f57a9
to
35d4e4f
Compare
if (pool[kNeedDrain] && !this.busy) { | ||
pool[kNeedDrain] = false | ||
pool.emit('drain') | ||
} |
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.
why this if was removed?
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.
I don't understand the question.
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.
you removed the if (queue.isEmpty())
check. Why?
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.
I replaced it with !this.busy
which contains the same check through this[kPending] > 0
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.
the empty check is moved to just if (pool[kClosedResolve] && queue.isEmpty()) {
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
No description provided.