jonschlinkert / delete-empty

Recursively delete all empty folders in a directory and child directories.
MIT License
42 stars 10 forks source link

Add dry run feature #9

Closed treble-snake closed 6 years ago

treble-snake commented 6 years ago

I'd like to propose new dry run feature I found myself in need of. Basically, it's an option to tell the tool to print found empty directories without removing them. I added --dry-run command line argument to the cli.

Also, I updated tests - adding path.normalize allows them to pass on Windows too, so it can be added to the Travis configs.

jonschlinkert commented 6 years ago

Nice work! I like the idea of adding dry-run, looks like a very well done pr, thank you!

@doowb?

doowb commented 6 years ago

Nice! I like this idea too. I have a few linting comments and some recommendations that I'll put in a review.

treble-snake commented 6 years ago

@doowb thanks for your review! changes committed :) I can squash the commits if you like.

doowb commented 6 years ago

@treble-snake did you see this comment? I was thinking more about the cli part when suggesting minimist, but as @jonschlinkert points out, the main purpose of this module is to use it as a library in other modules. I think his suggested change is better than mine.

treble-snake commented 6 years ago

@doowb Sorry, I missed it. Yeah, makes sense. I'm goning to undo the last commit and change the first one.

treble-snake commented 6 years ago

@doowb @jonschlinkert Updated. But there's a weird build failure, seems like Travis issue. Could you re-run it?

doowb commented 6 years ago

I'm not able to re-run it, but it looks like it's just a travis issue. The code looks good to me, so if @jonschlinkert doesn't have any other comments, I think we can merge.

jonschlinkert commented 6 years ago

But there's a weird build failure, seems like Travis issue. Could you re-run it?

done! yeah you were both correct, it was just a weird travis error. I'll merge as soon as I get a chance, unless @doowb has time to do it first. thanks to both of you!