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

readdir docs don’t mention when options argument was added #12299

Closed
chris-morgan opened this issue Apr 10, 2017 · 3 comments
Closed

readdir docs don’t mention when options argument was added #12299

chris-morgan opened this issue Apr 10, 2017 · 3 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

@chris-morgan
Copy link

fs.readdir(path[, options], callback)

History
Version Changes
v7.0.0 The callback parameter is no longer optional. Not passing it will emit a deprecation warning.
v0.1.8 Added in: v0.1.8

options was added in Node 6.0.0 by #5616. But it’s not mentioned, which led me to writing generic code that passed through undefined for options, which naturally made it fall apart on the Node 4 LTS release that doesn’t support options and so perceived it as just an undefined callback.

This really worries me. If something like this can change without any note in the history, what else is lacking history of changes that will come and bite me later? I no longer trust this documentation. At least https://nodejs.org/docs/latest-v4.x/api/fs.html#fs_fs_readdir_path_callback is still there, I’ll just need to remember to work there.

@vsemozhetbyt vsemozhetbyt added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. v4.x labels Apr 10, 2017
@joyeecheung joyeecheung added the good first issue Issues that are suitable for first-time contributors. label Apr 10, 2017
@joyeecheung
Copy link
Member

joyeecheung commented Apr 10, 2017

The per-item changes in the documentation is added in #11489, which happened much later than the v6.0.0 release, so it's very likely there would be something falling through the cracks. The differences between v4 and v6 are also documented in
https://github.com/nodejs/node/wiki/Breaking-changes-between-v4-LTS-and-v6-LTS.

I agree the missing documentation for previous changes would confuse users, and it definitely needs improvement, though I believe the documentation would be more reliable in future releases, now that we have a system like this in place.

Labeled as a good first contribution, feel free to open a PR to fix this!

(Also, maybe we can open a meta issue to hunt for more missing changelog items? Should not be that hard if we go through the semver-major PRs)

@gibfahn
Copy link
Member

gibfahn commented Apr 10, 2017

Should not be that hard if we go through the semver-major PRs

We'd have to look at semver-minor PRs as well (and check whether/how far they got backported).

@shubheksha
Copy link
Contributor

I'll take this up!

@aqrln aqrln closed this as completed in 4ac3ef5 Apr 12, 2017
evanlucas pushed a commit that referenced this issue Apr 25, 2017
Document that the `options` parameter of `fs.readdir()` was added
in Node.js 6.0.0.

PR-URL: #12312
Fixes: #12299
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this issue May 1, 2017
Document that the `options` parameter of `fs.readdir()` was added
in Node.js 6.0.0.

PR-URL: #12312
Fixes: #12299
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
evanlucas pushed a commit that referenced this issue May 2, 2017
Document that the `options` parameter of `fs.readdir()` was added
in Node.js 6.0.0.

PR-URL: #12312
Fixes: #12299
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
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

Successfully merging a pull request may close this issue.

5 participants