Add tag_to_cellar method #12980
Add tag_to_cellar method #12980
Conversation
|
Review period will end on 2022-03-11 at 06:34:33 UTC. |
BrewTestBot
added
the
waiting for feedback
Library/Homebrew/software_spec.rb
Outdated
| @@ -504,14 +504,19 @@ def root_url(var = nil, specs = {}) | |||
| end | |||
| end | |||
|
|
|||
| sig { params(tag: Utils::Bottles::Tag).returns(T::Boolean) } | |||
| def compatible_locations?(tag: Utils::Bottles.tag) | |||
| sig { params(tag: Utils::Bottles::Tag).returns(String) } | |||
There was a problem hiding this comment.
The type signature here is wrong for the return value and causing it to fail in testing. Sometimes it will be a Symbol (:any, :any_skip_relocation), but other times it will be a path (/usr/local etc.). What's the right type signature that covers both of those?
There was a problem hiding this comment.
Perhaps you want T.any(Symbol, Pathname)?
There was a problem hiding this comment.
Testing this locally, it looks like the return values are either Symbol or String, so I guess that's what we have to use here. Thanks for the pointer about using T.any!
danielnachun
force-pushed
the
add_tag_to_cellar
branch
from
735cf15
to
efb73c3
Compare
danielnachun
force-pushed
the
add_tag_to_cellar
branch
from
efb73c3
to
2c4ad58
Compare
BrewTestBot
removed
the
waiting for feedback
|
Review period ended. |
There was a problem hiding this comment.
Thanks @danielnachun! Happy to be merged when you're ready.
This is a relatively minor change that extract the code we currently use to get the cellar from the tag from
compatible_locations?into its own method so we can call it directly, and refactorscompatible_locations?to use that method, to avoid redundant code. Basic unit tests have also been added for both methods. More complex testing may be difficult without more refactoring becauseHOMEBREW_CELLARandHOMEBREW_PREFIXare constants that I didn't see an obvious way to override.