FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: clarify arguments on fs.create{Read|Write}Stream by yosuke-furukawa · Pull Request #2023 · nodejs/node · GitHub

/ node Public

doc: clarify arguments on fs.create{Read|Write}Stream - #2023

Closed
yosuke-furukawa wants to merge 1 commit into
nodejs:masterfrom
yosuke-furukawa:fix_fs_stream_doc
Closed

doc: clarify arguments on fs.create{Read|Write}Stream#2023
yosuke-furukawa wants to merge 1 commit into
nodejs:masterfrom
yosuke-furukawa:fix_fs_stream_doc

Conversation

Copy link
Copy Markdown
Member

Lots of discussions on this PR, #1998
But I would like to clarify the fs.create{Read|Write}Stream arguments to prevent misleading the issue #1981

ChALkeR commented Jun 20, 2015

Copy link
Copy Markdown
Member

I don't think that's the most important part of the doc, there is no reason for it to be bold.
Apart from that, are we going to put a label (except function) in all options descriptions all over the docs? That would look a bit clumsy to me.

cjihrig commented Jun 20, 2015

Copy link
Copy Markdown
Contributor

I agree with @ChALkeR.

seishun commented Jun 20, 2015

Copy link
Copy Markdown
Contributor

Apart from that, are we going to put a label (except function) in all options descriptions all over the docs? That would look a bit clumsy to me.

Exactly, so better just to accept functions everywhere. No one will likely ever do it or care, but if someone ever does find a use case for it, they won't be frustrated.

There is likely no use case for passing an Array or a Buffer as the options object, and it looks just as confusing, yet it's allowed.

mscdex added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Jun 20, 2015

targos commented Jun 20, 2015

Copy link
Copy Markdown
Member

I feel like there is a misunderstanding here. The original issue (#1981) has nothing to do with the use of functions as option objects. A function is being wrongly used as if fs.createWriteStream accepted a callback.

ChALkeR commented Jun 20, 2015

Copy link
Copy Markdown
Member

@targos #1982 (comment)

Copy link
Copy Markdown
Member Author

@ChALkeR I removed the bold style.

That would look a bit clumsy to me.

Yes, I agree.
My opinion is "if someone misunderstand the API, we should put a notice in API doc or improve API usage".

So currently we don't need to put the label "(except function)" in all API docs.

so better just to accept functions everywhere

-1, I have a same opinion with @targos .
We should prevent that users misunderstand that "wow, every function can accept callback!"

And we should discuss how document is improved in the docs repo. https://github.com/nodejs/docs

seishun commented Jun 20, 2015

Copy link
Copy Markdown
Contributor

We should prevent that users misunderstand that "wow, every function can accept callback!"

The documentation wouldn't change in this case, so I don't see why the users would suddenly start trying to pass functions.

Copy link
Copy Markdown
Contributor

I'm going to close this for now, as it seems like there is some pushback and it has sat here for a while. cc @nodejs/documentation if they want to do anything with it?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL