-
Notifications
You must be signed in to change notification settings - Fork 337
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
added package fam removal func, and tests #1252
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.
some notes
src/rez/cli/rm.py
Outdated
"-f", "--family", | ||
help="remove the entire specified package family (eg 'python'). This " | ||
"is only supported if the family is empty, or all its packages are " | ||
"hidden") |
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 ought to avoid archiving a family if it contains any packages, including hidden packages.
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.
agreed, updating
from rez.package_remove import remove_package_family | ||
|
||
if opts.dry_run: | ||
parser.error("--dry-run is not supported with --family") |
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.
dry run could probably at least tell you if it would allow proceeding with an archival if it weren't a dry run?
Have you considered a |
That's it, it's a dangerous op. I figured it's better to make the user do that themselves, than accidentally delete a whole fam. |
And what about a |
Still dangerous but |
-added force option -only allow rm of truly empty packages when force=false
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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 will have good use for this. Agreeing with the proposed and accepted changes.
fixes #1248