holmgr / cargo-sweep

A cargo subcommand for cleaning up unused build files generated by Cargo
MIT License
729 stars 31 forks source link

Only show toolchain list once when using `--installed` #88

Closed marcospb19 closed 1 year ago

marcospb19 commented 1 year ago

Fixes #86.

marcospb19 commented 1 year ago

EDIT: see comment below from Joshua.

I force pushed the fix to a test, it was panicking in the main function.

One thing I noticed is that https://github.com/holmgr/cargo-sweep/blob/a36602ffb06c2b62f35d246e22dc33b5163a69b0/src/main.rs#L157 is the only place where main can panic, in the other 9 places, the error is outputted and the program exits, maybe we should get rid of this one panic?

jyn514 commented 1 year ago

I force pushed the fix to a test, it was panicking in the main function.

We cleared this up in DMs, this was a panic in the test suite, not cargo-sweep itself.

marcospb19 commented 1 year ago

I think I'll wait for #78 to merge, to avoid giving him new conflicts again, and to reuse the project structure he created to test -r to test this one right here.

marcospb19 commented 1 year ago

Changing to draft while we wait on #78 to be merged to reuse some of the testing code.

marcospb19 commented 1 year ago

Excuse me @jyn514, could you please update the label S-waiting-on-author to S-waiting-on-review?

jyn514 commented 1 year ago

Sorry for the long delay. I am taking a semi-permanent break from github and don't know when I'll have time to maintain cargo-sweep.

@marcospb19 you've done a lot of work on cargo-sweep in the past and I trust your judgement - are you interested in becoming a maintainer? :)

jyn514 commented 1 year ago

In the meantime I'm happy to merge this as soon as you fix the conflicts.

marcospb19 commented 1 year ago

@jyn514

Sorry for the long delay. I am taking a semi-permanent break from github and don't know when I'll have time to maintain cargo-sweep.

Have a great break! And thanks for everything you've done.

@marcospb19 you've done a lot of work on cargo-sweep in the past and I trust your judgement - are you interested in becoming a maintainer? :)

I feel flattered to have your trust!

Yeah, I'm down! I believe the next good step would be to check out cargo-clean-all, see if both overlap and where are they going with the project.

In the meantime I'm happy to merge this as soon as you fix the conflicts.

Ok, I'll do that!

marcospb19 commented 1 year ago

Rebase done, conflicts solved.

marcospb19 commented 1 year ago

Tests failing due to that bad regex on that specific test... let me see what I can do.

Update to self: in macOS, there's a additional .o file being removed.

marcospb19 commented 1 year ago

Done by last commit! Clippy is failing due to a proc-macro2 update issue, which should be fixed by another PR.

jyn514 commented 1 year ago

Yeah, I'm down! I believe the next good step would be to check out cargo-clean-all, see if both overlap and where are they going with the project.

sounds perfect! check your email, you should have an invite :)