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
Add per-os versions to Cask::Cask#to_h
#11915
Conversation
Review period will end on 2021-08-25 at 16:22:45 UTC. |
Something else: we may need the same thing for arm/intel. I haven't given this any thought, yet. I wonder if we need a bottle-like OS syntax (that is, |
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 work here, this approach makes sense to me.
Move the resetting of `MacOS.version` to an `ensure` block. Fix the tests by adding a test OS and new fixture that has a different version for that test OS.
I ended up changing so that the only versions shown are the ones that differ. I did this because it seems a bit unnecessary to need to list all 8+ possible versions for every cask when most casks don't even have differing versions. Also, it made the tests easier. So, here's some of the output now of {
"formulae": [
],
"casks": [
{
"token": "onyx",
...
"version": "3.9.8",
"versions": {
"catalina": "3.8.7",
"mojave": "3.6.8",
"high_sierra": "3.4.9",
"sierra": "3.3.1",
"el_capitan": "3.1.9",
"yosemite": "3.0.2"
},
...
},
{
"token": "docker",
...
"version": "3.6.0,67351",
"versions": {
},
...
}
]
} |
Nice, I thought the same 🎉 |
Review period ended. |
I'm going to merge this so something can get through to formulae.brew.sh. I still want to look into the arm side of things (and, eventually, handling formula/cask dependencies that may differ on each version) but I'd rather get the cask JSON stuff set up and ready to be tested by maintainers |
🤦 this doesn't actually solve the problem I'm having in #11859. The issue is that the versions API doesn't have the right versions. I think this is probably still the right way to do it and I'll just need to follow up in formulae.brew.sh so update the versions API, too. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This PR adds a
versions
key to the cask JSON output. This key contains a hash mapping macOS versions to cask versions. Some casks, likeonyx
, have differentversion
stanzas based on the OS version.Some things to note about the field. For now, it contains versions for all macOS versions present in
OS::Mac::Versions::SYMBOLS
, even if the cask version matches the default. Instead, I could make it so the only versions that show inversions
are the ones that don't match the defaultversion
field. Thoughts?I'd also appreciate suggestions for naming these fields/methods. I like
version
andversions
for the JSON fields because it implies "default" and "all" versions. However,Cask::Cask#versions
already exists to find the installed versions, so I called the methodos_versions
. Does that imply that it returns available OS versions rather than per-os-cask-versions? And is consistency between the JSON fields and the method names more important? I'll keep thinking about it.For some context, see #11859