Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes cask audit performance by reducing redundant livecheck URL fetches from up to three times down to one in most cases. This is achieved by introducing a caching mechanism for the livecheck version audit result.
- Adds
@livecheck_resultinstance variable to cache livecheck audit results - Modifies
audit_livecheck_versionto cache and reuse results across multiple audit methods - Updates
audit_hosting_with_livecheckto callaudit_livecheck_versiondirectly instead of accepting it as a parameter - Adds early return optimization in
audit_livecheck_https_availabilityto skip unnecessary checks when livecheck already uses HTTPS and succeeds
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
40ae65b to
45c08f7
Compare
MikeMcQuaid
approved these changes
Dec 23, 2025
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks good once small nit addressed! Can self-merge without rereview.
The existing livecheck-related cask audits can end up fetching the livecheck URL four times and this can cause problems if a server has sensitive rate limiting. This modifies the `livecheck_version` audit to cache the result that's also used in the `hosting_with_livecheck` and `livecheck_https_availability` audits. This will avoid a duplicate fetch and can avoid another if the livecheck URL uses HTTPS and the check doesn't fail. Most casks meet that criteria, so this should be a notable improvement. There's still room for improvement, as the `min_os` audit calls `cask_sparkle_min_os`, which can also fetch the livecheck URL. It would be ideal if we could cache the livecheck content to avoid this duplicate fetch but I don't think that's possible with the current setup, so I'll have to revisit that later.
45c08f7 to
9ba98a6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
brew lgtm(style, typechecking and tests) with your changes locally?The existing livecheck-related cask audits can end up fetching the livecheck URL four times and this can cause problems if a server has sensitive rate limiting (e.g., Homebrew/homebrew-cask#241429 (comment)). This modifies the
livecheck_versionaudit to cache the result that's also used in thehosting_with_livecheckandlivecheck_https_availabilityaudits. This will avoid a duplicate fetch and can avoid another if the livecheck URL uses HTTPS and the check doesn't fail. Most casks meet that criteria, so this should be a notable improvement.There's still room for improvement, as the
min_osaudit callscask_sparkle_min_os, which can also fetch the livecheck URL. It would be ideal if we could cache the livecheck content to avoid this duplicate fetch but I don't think that's possible with the current setup, so I'll have to revisit that later. [I have some in-progress work to add caching to livecheck and I may be able to leverage that in the future as a way of addressing this.]