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

Disable rimraf globbing #280

Merged
merged 1 commit into from
Sep 29, 2016
Merged

Conversation

distracteddev
Copy link
Contributor

Its important to pass the options through as ensuring symmetry between the fs.extra#remove and rimraf APIs means I don't need to include rimraf AND fs-extra, just for the sake of using rimraf with specific options.

One option that is particularly useful and quite commonly used is disableGlob.

@jprichardson
Copy link
Owner

I appreciate your thoughts here.

Its important to pass the options through as ensuring symmetry between the fs.extra#remove and rimraf

This is assuming that I felt symmetry was important. I'm strongly considering removing rimraf since it feels like rimraf is getting bloated.

I would accept a PR to set {disableGlob: false} to rimraf though since fs-extra doesn't allow this anyways.

@distracteddev
Copy link
Contributor Author

Heh, valid point re: symmetry.

@distracteddev distracteddev changed the title Pass options from remove through to rimraf Disable rimraf globbing Sep 28, 2016
@distracteddev
Copy link
Contributor Author

@jprichardson PR updated with globbing explicitly disabled

@jprichardson
Copy link
Owner

Thank you. Please squash changes. Will merge after Oct 1 and subsequently publish fs-extra@1.0.0.

@distracteddev
Copy link
Contributor Author

@jprichardson done :)

@jprichardson
Copy link
Owner

Thank you! I will merge after Oct 1st so that I can publish this along with dropping support for Node v0.10.

@jprichardson
Copy link
Owner

Actually, doesn't hurt for me to merge now. Will publish after Oct 1st :)

@jprichardson jprichardson merged commit aec5548 into jprichardson:master Sep 29, 2016
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

Successfully merging this pull request may close these issues.

2 participants