Skip to content
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

Closed
joyeecheung opened this issue Jun 7, 2018 · 9 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Jun 7, 2018

  • Version: master
  • Subsystem: fs, doc

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 mentions

Synchronous version of fs.read(). Returns the number of bytesRead.

Which 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 the fs.readSync() docs we explicitly say:

Synchronous version of fs.read(). Returns the number of bytesRead.

For detailed information, see the documentation of fs.read().

And do the same for other fs.*Sync APIs if applicable.

Refs: #21193

@joyeecheung joyeecheung added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors. labels Jun 7, 2018
@iwko
Copy link
Contributor

iwko commented Jun 7, 2018

@joyeecheung May I take this one?

@joyeecheung
Copy link
Member Author

@iwko Sure, go ahead!

@BeniCheni
Copy link
Contributor

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.

@TimothyGu
Copy link
Member

@BeniCheni, hmm, did you see #21243 (linked above by GitHub automatically)?

@BeniCheni
Copy link
Contributor

Sorry about missing #21243. Closed my own duplicated PR.

Trott pushed a commit to Trott/io.js that referenced this issue Jul 9, 2018
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]>
targos pushed a commit that referenced this issue Jul 10, 2018
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]>
@lirantal
Copy link
Member

@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

@adarshsaraogi
Copy link

Can i work on this issue

@iwko
Copy link
Contributor

iwko commented Oct 24, 2018

@adarshsaraogi this issue is alread resolved and merged

@Trott
Copy link
Member

Trott commented Nov 11, 2018

(As always, if I'm closing in error, please comment or re-open. Thanks.)

@Trott Trott closed this as completed Nov 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

7 participants