-
Notifications
You must be signed in to change notification settings - Fork 104
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 stream output for pruning #566
Add stream output for pruning #566
Conversation
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.
The feature looks good and the tests too. In terms of readability, we'll prefer to put the if
statement outside the assert
to lower the complexity of the logic and make it more readable. The else True
becomes then unnecessary.
After those small fixes, we can merge :) thanks for the PR!
Indeed this seems like a good idea, we'll have a consistent api :) |
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
Co-authored-by: Gabriel de Marmiesse <gabrieldemarmiesse@gmail.com>
070bf6e
into
gabrieldemarmiesse:master
Resolves #396
Not sure if we need to update this? #499
Will complete the overloading for other pruning, such as for Docker Image, Network, Buildx, etc. when this one gets approved.