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
cask: audit for minimal OS version in sparkle feeds #14060
cask: audit for minimal OS version in sparkle feeds #14060
Conversation
Review period will end on 2022-11-01 at 00:00:00 UTC. |
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 idea
Co-authored-by: Bo Anderson <mail@boanderson.me>
Love this because it is often forgotten until something breaks. |
The recent iTerm update is exactly what motivated me here. If you have other things people forget, don't hesitate to ask me or make your own audit for it. |
Review period ended. |
Thanks @SMillerDev! |
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.
LGTM
return unless cask.livecheckable? | ||
return unless cask.livecheck.strategy == :sparkle | ||
|
||
out, _, status = curl_output(cask.livecheck.url) |
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.
out, _, status = curl_output(cask.livecheck.url) | |
out, _, status = curl_output("--fail", "--silent", "--location", cask.livecheck.url) |
So that it will detect non-200 codes, follow redirects etc.
|
||
return if xml.blank? | ||
|
||
item = xml.get_elements("//rss//channel//item").first |
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.
item = xml.get_elements("//rss//channel//item").first | |
item = xml.elements["//rss//channel//item"] |
Just for consistent code style with below. elements[...]
should be the same as get_elements(...).first
.
Doesn't really matter though.
item = xml.get_elements("//rss//channel//item").first | ||
return if item.blank? | ||
|
||
min_os = item.elements["sparkle:minimumSystemVersion"].text |
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.
min_os = item.elements["sparkle:minimumSystemVersion"].text | |
min_os = item.elements["sparkle:minimumSystemVersion"]&.text |
If there's no minimum version, then this will be nil.
min_os = item.elements["sparkle:minimumSystemVersion"].text | ||
return if min_os.blank? | ||
|
||
min_os_string = OS::Mac::Version.new(min_os).strip_patch |
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.
min_os_string = OS::Mac::Version.new(min_os).strip_patch | |
begin | |
min_os_string = OS::Mac::Version.new(min_os).strip_patch | |
rescue MacOSVersionError | |
return | |
end | |
In case the source returns an invalid macOS version, or one we don't support (e.g. Yosemite and earlier).
Oops sorry @Bo98! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This should prevent situations where we forget to update the macOS dependency if there is a new version required upstream