-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
doc: remove "note that" from fs doc #21646
Conversation
doc/api/fs.md
Outdated
a callback.) | ||
`fs.exists()` is deprecated, but `fs.existsSync()` is not. (The `callback` | ||
parameter to `fs.exists()` accepts parameters that are inconsistent with other | ||
Node.js callbacks. `fs.existsSync()` does not use a callback.) |
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 don't think brackets around the last sentance are needed, also.
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.
Brackets removed. Thanks!
doc/api/fs.md
Outdated
@@ -2149,7 +2145,7 @@ fs.mkdtemp(tmpDir, (err, folder) => { | |||
if (err) throw err; | |||
console.log(folder); | |||
// Will print something similar to `/tmpabc123`. | |||
// Note that a new temporary directory is created | |||
// A new temporary directory is created |
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: can fit on three lines:
// Will print something similar to `/tmpabc123`.
// A new temporary directory is created at the file system root
// rather than *within* the /tmp directory.
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.
Wrapped and reduced to 3 lines. Thanks.
doc/api/fs.md
Outdated
Note that it is unsafe to use `fs.write` multiple times on the same file | ||
without waiting for the callback. For this scenario, | ||
`fs.createWriteStream` is strongly recommended. | ||
It is unsafe to use `fs.write` multiple times on the same file without waiting |
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.
`fs.write`
-> `fs.write()`
?
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.
This and the other similar nit below addressed. Thanks!
doc/api/fs.md
Outdated
Note that it is unsafe to use `fs.write` multiple times on the same file | ||
without waiting for the callback. For this scenario, | ||
`fs.createWriteStream` is strongly recommended. | ||
It is unsafe to use `fs.write` multiple times on the same file without waiting |
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.
`fs.write`
-> `fs.write()`
?
Remove "note that" from the fs documentation, along with other minor nearby improvements. Before: Note that some characters are obscured by Strong Bad's head. After: Some characters are obscured by Strong Bad's head.
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
Landed in f545ae9 |
Remove "note that" from the fs documentation, along with other minor nearby improvements. Before: Note that some characters are obscured by Strong Bad's head. After: Some characters are obscured by Strong Bad's head. PR-URL: nodejs#21646 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Remove "note that" from the fs documentation, along with other minor nearby improvements. Before: Note that some characters are obscured by Strong Bad's head. After: Some characters are obscured by Strong Bad's head. PR-URL: #21646 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Remove "note that" from the fs documentation, along with other minor
nearby improvements.
Before:
After:
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes