-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Prevent users from uninstalling casks with dependents #21078
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
Conversation
Signed-off-by: botantony <[email protected]>
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.
Pull Request Overview
This pull request implements functionality to prevent users from uninstalling casks that have dependent casks installed, similar to the existing behavior for formulae.
Key changes:
- Extracted
DependentsMessageclass to a shared module for reuse between formula and cask uninstall operations - Added
check_dependent_casksmethod to verify cask dependencies before uninstallation - Integrated the dependency check into the cask uninstall command workflow
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
Library/Homebrew/dependents_message.rb |
New shared class extracted from uninstall.rb to format and display dependency error messages with Sorbet type signatures |
Library/Homebrew/uninstall.rb |
Removed the DependentsMessage class (now in separate file) and added require statement for the new module |
Library/Homebrew/cask/uninstall.rb |
Added check_dependent_casks method to identify and report casks with dependencies before uninstallation |
Library/Homebrew/cmd/uninstall.rb |
Integrated the dependency check into the uninstall command flow, halting execution if dependencies are found |
Library/Homebrew/test/cask/uninstall_spec.rb |
Added comprehensive test coverage for the new dependency checking functionality, including single/multiple casks and dependents |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rylan12
left a comment
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.
Nice!
Signed-off-by: botantony <[email protected]>
MikeMcQuaid
left a comment
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.
Thanks @botantony!
brew lgtm(style, typechecking and tests) with your changes locally?Closes #21067