-
Notifications
You must be signed in to change notification settings - Fork 29.5k
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
Clarify "fs.readFile", "fs.appendFile", "fs.writeFile" documentation regarding closing #7560
Comments
Here is the pull request: #7561 |
Emmm, so what is left before this change can be pulled?.. |
I have two LGTMs on the pull request. If current blocker is the latest comment from the fourth reviewer, could I get a second opinion on that? Currently I have doubts about whether the suggested change should be made, but if someone else agrees it would be for the best, I'll make it. |
@kibertoad For one, it’s a bit confusing to see comments that are only about the PR to pop up on the thread for the corresponding issue. ;) |
@addaleax True, but for some reason comments about the PR inside the PR are ignored :). What would be the more appropriate way to keep things progressing? |
@kibertoad I think the occasional “ping”/“bump”/… comment on a PR which hasn’t received feedback for a couple of days is pretty much okay. |
Ref: #7560 PR-URL: #7561 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Ref: #7560 PR-URL: #7561 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
"https://nodejs.org/api/fs.html#fs_fs_readfile_file_options_callback" states that
This is confusing for new users, because it implies that opened file should always be manually closed, leading to questions like this - http://stackoverflow.com/questions/21523890/close-file-after-fs-readfile-nulling
It should be mentioned that if "file" is specified as a filename, it will, indeed, be closed automatically.
The text was updated successfully, but these errors were encountered: