| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@jasnell @misterdjules As discussed I created a new pull request for autoClose feature. node-v0.x-archive/pull/25275 This PR can be landed on v5/v4/v0.12/v0.10 it would be really nice if i see this PR land :-) |
Sorry, something went wrong.
|
cc @nodejs/streams |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you use const for imports?
Sorry, something went wrong.
|
Basic premise looks alright to me. The checks in the test can be made a little tighter. |
Sorry, something went wrong.
|
@bnoordhuis Done! I updated the review comments given by you. |
Sorry, something went wrong.
|
Overall this is fine, but as a semver-minor it woud not be able to land in v0.10, v0.12 or v4.x. |
Sorry, something went wrong.
There was a problem hiding this comment.
Would like to see this reworded a bit to make it absolutely obvious that the default behavior is autoClose = true. Perhaps move the third sentence to the front?
Sorry, something went wrong.
|
LGTM with one nit on the docs |
Sorry, something went wrong.
|
In your PR message, you write When an instance of fs.WriteStream created with autoClose === true finishes,, but I think you mean to say === false? |
Sorry, something went wrong.
|
@jasnell @ronkorving Done! Updated the doc and commit message. Thanks! |
Sorry, something went wrong.
|
The PR message still says "true" instead of "false", sorry :) Commit message is 👍 |
Sorry, something went wrong.
There was a problem hiding this comment.
Indent errors.
Sorry, something went wrong.
|
@saquibkhan ... when you get a chance, please take a look at @bnoordhuis' comments, then please rebase and update :-) |
Sorry, something went wrong.
There was a problem hiding this comment.
I am using ! to test fd is not null
Sorry, something went wrong.
There was a problem hiding this comment.
if you're specifically testing for not null, perhaps assert(stream.fd !== null) would be better
Sorry, something went wrong.
|
@bnoordhuis @jasnell I have updated the code as per review comments. |
Sorry, something went wrong.
|
@saquibkhan "This branch has conflicts that must be resolved" |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you make this const.
Sorry, something went wrong.
|
@cjihrig @ronkorving @jasnell @bnoordhuis Updated the code as per the review comments.. LGTM |
Sorry, something went wrong.
|
LGTM pending CI |
Sorry, something went wrong.
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose option that behaves similarly to the autoClose option supported by fs.createReadStream and fs.ReadStream. When an instance of fs.createWriteStream created with autoClose === false finishes, it is not destroyed. Its underlying fd is not closed and it is the responsibility of the user to close it.
|
@ronkorving @jasnell @bnoordhuis @cjihrig Can we merge? |
Sorry, something went wrong.
Sorry, something went wrong.
|
unrelated failure in CI |
Sorry, something went wrong.
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose option that behaves similarly to the autoClose option supported by fs.createReadStream and fs.ReadStream. When an instance of fs.createWriteStream created with autoClose === false finishes, it is not destroyed. Its underlying fd is not closed and it is the responsibility of the user to close it. PR-URL: #3679 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose option that behaves similarly to the autoClose option supported by fs.createReadStream and fs.ReadStream. When an instance of fs.createWriteStream created with autoClose === false finishes, it is not destroyed. Its underlying fd is not closed and it is the responsibility of the user to close it. PR-URL: #3679 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Notable changes: * events: make sure console functions exist (Dave) #4479 * fs: add autoClose option to fs.createWriteStream (Saquib) #3679 * http: improves expect header handling (Daniel Sellers) #4501 * node: allow preload modules with -i (Evan Lucas) #4696 * v8,src: expose statistics about heap spaces (`v8.getHeapSpaceStatistics()`) (Ben Ripkens) #4463 * Minor performance improvements: - lib: Use arrow functions instead of bind where possible (Minwoo Jung) #3622 - module: cache stat() results more aggressively (Ben Noordhuis) #4575 - querystring: improve parse() performance (Brian White) #4675 PR-URL: #4742
Notable changes: * events: make sure console functions exist (Dave) #4479 * fs: add autoClose option to fs.createWriteStream (Saquib) #3679 * http: improves expect header handling (Daniel Sellers) #4501 * node: allow preload modules with -i (Evan Lucas) #4696 * v8,src: expose statistics about heap spaces (`v8.getHeapSpaceStatistics()`) (Ben Ripkens) #4463 * Minor performance improvements: - lib: Use arrow functions instead of bind where possible (Minwoo Jung) #3622 - module: cache stat() results more aggressively (Ben Noordhuis) #4575 - querystring: improve parse() performance (Brian White) #4675 PR-URL: #4742
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose option that behaves similarly to the autoClose option supported by fs.createReadStream and fs.ReadStream. When an instance of fs.createWriteStream created with autoClose === false finishes, it is not destroyed. Its underlying fd is not closed and it is the responsibility of the user to close it. PR-URL: nodejs#3679 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Notable changes: * events: make sure console functions exist (Dave) nodejs#4479 * fs: add autoClose option to fs.createWriteStream (Saquib) nodejs#3679 * http: improves expect header handling (Daniel Sellers) nodejs#4501 * node: allow preload modules with -i (Evan Lucas) nodejs#4696 * v8,src: expose statistics about heap spaces (`v8.getHeapSpaceStatistics()`) (Ben Ripkens) nodejs#4463 * Minor performance improvements: - lib: Use arrow functions instead of bind where possible (Minwoo Jung) nodejs#3622 - module: cache stat() results more aggressively (Ben Noordhuis) nodejs#4575 - querystring: improve parse() performance (Brian White) nodejs#4675 PR-URL: nodejs#4742
| Back | FazBrowse Home | New Git URL |
Add support to fs.createWriteStream and fs.WriteStream for an autoClose
option that behaves similarly to the autoClose option supported by
fs.createReadStream and fs.ReadStream.
When an instance of fs.WriteStream created with autoClose === false finishes,
it is not destroyed. Its underlying fd is not closed and it is the
responsibility of the user to close it.