-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: clean cache #216
feat: clean cache #216
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #216 +/- ##
==========================================
+ Coverage 65.37% 66.00% +0.63%
==========================================
Files 38 40 +2
Lines 5048 5413 +365
Branches 5048 5413 +365
==========================================
+ Hits 3300 3573 +273
- Misses 1229 1277 +48
- Partials 519 563 +44
|
Need #217 merged so that I can complete the implementation of mocking for testing the implementation of the command. |
0578669
to
5f1db14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove #[cfg(feature = "parachain")]
and support removing folders.
A nice-to-have feature would be the option to clean all with an all
option in the multiselect or a --all
flag. However, this feature is not essential for this PR. If it is not included, I will create a new issue with the good first issue
tag for new contributors.
Note: have wrapped the cliclack usage in a trait in an effort to enable testing of interactions and improve coverage in the pop-cli crate. Its a manual mock so isnt perfect, but does enable some testing of command logic. Note also that the cli should not be defined on the command as it is not state applicable to the command, but is fine for an initial iteration. This should be refactored into a command handler in the future which injects the required dependencies for a cleaner implementation. |
Why do we need to remove folders? To my knowledge no folders are created here.
Would be nice to have but afraid I dont have the bandwidth to add this now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should address the cli parameter on the command as a separate PR first, as it is a dependency rather than command state, before moving all the commands to this type of implementation. I think it will be much easier that way, at least in terms of reviews. |
Adds a simple command which allows a user to clean the cache by selecting those cached artifacts which are no longer required.
Note: uses 'clean' so that we can potentially add a default implementation of 'pop clean' to wrap 'cargo clean'. No real benefit apart from fewer characters to type.
Closes #78