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

Confusing description of traverseAll default value #15

Closed
l1bbcsg opened this issue Apr 29, 2019 · 3 comments
Closed

Confusing description of traverseAll default value #15

l1bbcsg opened this issue Apr 29, 2019 · 3 comments

Comments

@l1bbcsg
Copy link

l1bbcsg commented Apr 29, 2019

Package version: 6.0.0

The README reads:

  • traverseAll <Boolean> default: true
    • traverse all subdirectories, regardless of filter option. (When set to true, traverseAll produces similar behavior to the default behavior prior to v4.0.0. The current default of traverseAll: false is equivalent to the old noRecurseOnFailedFilter: true).

First, it says it's true by default.

Then it says "The current default of traverseAll: false" implicating it's false by default.

In the code it is in fact undefined, which is equivalent to false in this case.

If the parameter is not present, klaw-sync behaves as if it was false.

What is the intended default value of this parameter, true or false?
true seems more reasonable to me, but regardless, I believe default value should be indicated clearly in the docs and correspond to the actual behavior.

I also find it confusing that sister module node-klaw does not have this option at all.

@manidlou
Copy link
Owner

@l1bbcsg I agree the docs is not clear about traverseAll option. The default value for traverseAll is true. I will update the docs. Also, we figured this is a good approach to handle reading directories recursively while supporting filter option.

For node-klaw, we still need to so something about it! there are ongoing discussions on node-klaw repo.

jprichardson/node-klaw#11
jprichardson/node-klaw#17

@manidlou
Copy link
Owner

Closed via bc081f0

@l1bbcsg
Copy link
Author

l1bbcsg commented May 28, 2019

The actual default is still false though. See the linked line of code, the parameter is not set and therefore is undefined which then casts to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants