-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
More CaskLoader
improvements.
#16621
More CaskLoader
improvements.
#16621
Conversation
ece19a3
to
fca915c
Compare
fca915c
to
76d8d6c
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.
Looks good to me. Always nice to remove some code.
(!CoreTap.instance.installed? && Homebrew::API::Formula.all_formulae.key?(path.basename)) | ||
(!CoreTap.instance.installed? && Homebrew::API::Formula.all_formulae.key?(path.basename.to_s)) | ||
paths << formula_path | ||
end | ||
if cask_path.exist? || | ||
(!CoreCaskTap.instance.installed? && Homebrew::API::Cask.all_casks.key?(path.basename)) | ||
(!CoreCaskTap.instance.installed? && Homebrew::API::Cask.all_casks.key?(path.basename.to_s)) |
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 guess this somehow got coerced to strings before. Otherwise, I'm not really sure how this ever worked.
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 it's because we have a Pathname#to_str
monkey-patch.
@path = path | ||
@tap = Homebrew::API.tap_from_source_download(path) | ||
@tap = Tap.from_path(path) || Homebrew::API.tap_from_source_download(path) |
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 this might even be a touch faster here. The Homebrew::API.tap_from_source_download
method is quite slow when it gets called a lot in commands like brew readall
. This change in logic means it will get called less in that scenario though I plan on opening a PR soon to move from Pathname#relative_path_from
in that method to a regex to speed things up anyway.
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?