Added the --all flag #91
Conversation
src/cli.rs
Outdated
| @@ -78,6 +78,10 @@ pub struct Args { | |||
| #[arg(short, long, value_name = "DAYS")] | |||
| time: Option<u64>, | |||
|
|
|||
| /// Remove everything | |||
There was a problem hiding this comment.
This might be misleading, because cargo sweep doesn't remove everything as cargo clean does, but I can't come up with something short and better than this, opinions @jyn514?
There was a problem hiding this comment.
"Remove all versioned artifacts", maybe. The flag won't remove examples, the incremental directory, or various misc things created by proc-macros or build scripts.
Honestly given that I wonder if this flag still makes sense to add? I think people will expect it to behave differently than it does, maybe we should recommend find . -name target | xargs rm -r or something instead.
There was a problem hiding this comment.
The flag won't remove examples, the incremental directory, or various misc things created by proc-macros or build scripts.
I think this is exposing a communication problem in the tool, and it's not just related to --all, from the README:
A cargo subcommand for cleaning up unused build files generated by Cargo.
From reading this alone, we can't infer that cargo-sweep can't clean all of this you just described, and that's relevant when using other flags too, like --installed, --time and --maxsize.
So it's not just a confusion about --all.
I think the right approach to this would be to clarify on README and the --help description what it can sweep.
We can describe what files are "sweepable" and the current capabilities of cargo-sweep, before the usage examples, with that properly communicated, I think that the --all behavior will be obvious to be expected.
Even after addressing this, we can still argue if the --all flag would be confusing, but I think it wouldn't.
There was a problem hiding this comment.
Ok, that makes sense to me. @Max-Weissman do you mind making that change to the readme and help as part of this PR?
There was a problem hiding this comment.
Sure! I'll get started on that
There was a problem hiding this comment.
@jyn514
I read through the code and updated the readme to what I think cargo-sweep is doing. I didn't change the help because to give a longer description I'd have to specify which artifacts each tag is referring to which might be a bit wordy. But I can add that too if you want.
|
Noooooooooooo, I rebased your branch and it actually closed your PR. @Max-Weissman are you able to re-open it? |
|
I reopened the PR at #122. Sorry for my mistake, I wanted to push changes on your However, there are two |
Implemented the delete everything as --time 0.