-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add cli #1
Comments
That sounds like a great idea! I have to definitely look into that. That was the initial idea of this module, but it got lost somewhere when I was developing it. Would it make more sense to separate it to it's own standalone module though? Like 'file-wipe-cli'? Would like to hear your opinion on this. |
@simonlovesyou So the whole cli vs non-cli separation is super opinionated. I could care less, but some people don't like the idea of a node module that is packaged with the cli if they never use the global install. To be the safest split them up into separate repos where the cli does nothing but argument checking, a |
Actually, I'm not vibing on the code right now, count me out. I'm interested in the space of secure file deletion, but not this method. Thanks for the merge though on the testing work 👍 |
What's not vibing with you? I don't want my code to detest other potential contributors in the future, or you for that matter, so a rewrite might be in order. |
It's not very modular. So for instance look into doing this with glob. You're going to have to call your functions in a I could be convinced. When you get an opportunity try out the file globbing, I think you'll have to take care of the refactoring by nature when doing that. And for the tests, you'll actually need to test your es5 and es6 versions that they actually delete a file. I pushed my changes and your tests are not passing on my system. Take a look around in my tests and let me know what you think. They should def be merged into one file after the issues are figured out. |
Have you considered adding a cli for this? I can imagine this being popular as a replacement for
rm
. If you add a cli for it, you could even provide users with an easyalias rm="wipe"
tip in the readme file.The text was updated successfully, but these errors were encountered: