-
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: explicitly mention that the explanation of an synchronous fs API live in the docs of the async API #21197
Comments
@joyeecheung May I take this one? |
@iwko Sure, go ahead! |
Since didn't see any open PR fo 15 days from last comment so I opened #21479 to try this "good first issue" labeled issue. Thanks for your time to review. |
@BeniCheni, hmm, did you see #21243 (linked above by GitHub automatically)? |
Sorry about missing #21243. Closed my own duplicated PR. |
Add links to async methods and make wording consistent. PR-URL: nodejs#21243 Refs: nodejs#21197 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Jamie Davis <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Add links to async methods and make wording consistent. PR-URL: #21243 Refs: #21197 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Jamie Davis <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
@joyeecheung seems like we can close this issue having @iwko's PR landed already, unless there are other areas we want to make the change in...? What do you think about zlib? the section for all the methods do specify the connection between the sync and async version of them here: https://nodejs.org/api/zlib.html#zlib_convenience_methods |
Can i work on this issue |
@adarshsaraogi this issue is alread resolved and merged |
(As always, if I'm closing in error, please comment or re-open. Thanks.) |
In a lot of cases, the documentation of a synchronous fs API only links to the aysnc version of that API in order to reduce duplicated texts.
For example,
fs.readSync()
only mentionsWhich could be confusing to beginners since it does not explicitly mention that if they want to see a detailed explanation of the arguments, they should click the link of
fs.read()
. It would be more friendly if, for example, in thefs.readSync()
docs we explicitly say:And do the same for other
fs.*Sync
APIs if applicable.Refs: #21193
The text was updated successfully, but these errors were encountered: