Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables brew bundle to use a preinstalled mas binary from the system PATH, rather than requiring mas to be installed via Homebrew formula. This aligns the handling of mas with other external dependencies like cargo, go, flatpak, and vscode.
Changes:
- Added
which_masmethod that useswhich("mas", ORIGINAL_PATHS)to locate themasbinary - Updated
mas_installed?to check for the presence of amasbinary viawhich_masinstead of checking for the formula - Modified
mac_app_store_installer.rbto useBundle.which_maswhen checking for outdated apps - Modified
mac_app_store_dumper.rbto useBundle.which_maswhen listing installed apps
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Library/Homebrew/bundle.rb | Added which_mas method and updated mas_installed? to use it, following the pattern of other external tools |
| Library/Homebrew/bundle/mac_app_store_installer.rb | Updated outdated_app_ids method to use Bundle.which_mas instead of hardcoded "mas" command |
| Library/Homebrew/bundle/mac_app_store_dumper.rb | Updated apps method to use Bundle.which_mas instead of hardcoded "mas" command |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e311bc1 to
be8e6ff
Compare
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks great, thanks @bevanjkay!
|
Just fixing a couple of things, so not ready for merge just yet |
|
@bevanjkay cool, just self-merge when you're happy! |
be8e6ff to
9718336
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9718336 to
9e3f6f1
Compare
brew lgtm(style, typechecking and tests) with your changes locally?I used
codexto help discover the correct fix.This PR aligns the handling of
maswith other external dependencies forbrew bundle, allowing the use of an externally installed binary, such asmasinstalled via upstream's package installer. The only downside I can see is that it is possible that the provided package could be use an interface that is incompatible if it is running an old version, but I'm not sure it is worth the extra effort to include version detection here, and this issue is not unique tomas.