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: clarify arguments on fs.create{Read|Write}Stream #2023

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

yosuke-furukawa
Copy link
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
Copy link
Member

ChALkeR commented Jun 20, 2015

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
Copy link
Contributor

cjihrig commented Jun 20, 2015

I agree with @ChALkeR.

@seishun
Copy link
Contributor

seishun commented Jun 20, 2015

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 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
Copy link
Member

targos commented Jun 20, 2015

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
Copy link
Member

ChALkeR commented Jun 20, 2015

@targos #1982 (comment)

@yosuke-furukawa
Copy link
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
Copy link
Contributor

seishun commented Jun 20, 2015

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.

@brendanashworth
Copy link
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?

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