Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uplib: fix stream as context is redundant #35728
Open
+2
−2
Conversation
|
Review requested: |
Codecov Report
@@ Coverage Diff @@
## master #35728 +/- ##
=======================================
Coverage 96.40% 96.40%
=======================================
Files 222 223 +1
Lines 73682 73685 +3
=======================================
+ Hits 71033 71036 +3
Misses 2649 2649
Continue to review full report at Codecov.
|
lib/internal/streams/from.js
Outdated
| // being called before last iteration completion. | ||
| let reading = false; | ||
|
|
||
| // needToClose boolean if iterator needs to be explicitly closed | ||
| // Flag for iterator when needs to be explicitly closed |
lpinca
Oct 21, 2020
Member
Suggested change
// Flag for iterator when needs to be explicitly closed
// Flag for iterator when needs to be explicitly closed.
Suggested change
| // Flag for iterator when needs to be explicitly closed | |
| // Flag for iterator when needs to be explicitly closed. |
|
The subsystem in commit title should be |
043cd17
to
daa264f
|
Updated @lpinca |
|
lgtm |
lib/internal/streams/from.js
Outdated
| // being called before last iteration completion. | ||
| let reading = false; | ||
|
|
||
| // needToClose boolean if iterator needs to be explicitly closed | ||
| // Flag for iterator when needs to be explicitly closed. |
Trott
Oct 21, 2020
Member
Suggested change
// Flag for iterator when needs to be explicitly closed.
// Flag for when iterator needs to be explicitly closed.
Suggested change
| // Flag for iterator when needs to be explicitly closed. | |
| // Flag for when iterator needs to be explicitly closed. |
yashLadha
Oct 21, 2020
Author
Contributor
Made suggested changes 👍
Made suggested changes
|
The commit message ( |
Using the variable name in the comment and justifying the type seems redundant to me and instead it should defined the entity which it is acting, like in our case it is acting as a flag to control the flow in streams.
daa264f
to
2aa775a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Using the variable name in the comment and justifying the type seems
redundant to me and instead it should defined the entity which it is
acting, like in our case it is acting as a flag to control the flow in
streams.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes