Tweak cask-source API handling #14439
Merged
Conversation
This file contains 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
- Use raw.githubusercontent.com to download cask source rather than formulae.brew.sh. This allows us to remove these files - output the tap's current `HEAD` for both formulae and cask JSON - use this `HEAD` for the cask-source API to get the exact file on raw.githubusercontent.com rather than just whatever is newest (which is what the previous API did) - set the `Tap` correctly when creating a `Cask` from the API - if the `formula.json` file exists: print its modified time include `brew config` - memoize `tap.git_head` as we'll be calling it a lot in the same process with the same value
MikeMcQuaid
added
the
critical
|
Review period ended. |
BrewTestBot
approved these changes
Jan 26, 2023
Rylan12
reviewed
Jan 27, 2023
There was a problem hiding this comment.
Thanks for opening this!
I think the cask source method should live in Homebrew::API::Cask since the modules within Homebrew::API are meant to correspond to actual formulae.brew.sh API endpoints. If we're planning on removing /api/cask-source, we should also remove Homebrew::API::CaskSource.
I'm away for a family event this weekend so won't have as much time to play with it, but I'll try to push those changes and test it out a bit when I have time
| return FromContentLoader.new(cask_source).load(config: config) | ||
| end | ||
|
|
||
| # convert generic string replacements into actual ones | ||
| json_cask[:artifacts] = json_cask[:artifacts].map(&method(:from_h_hash_gsubs)) | ||
| json_cask[:caveats] = from_h_string_gsubs(json_cask[:caveats]) | ||
|
|
||
| Cask.new(token, source: cask_source, config: config) do | ||
| tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/") |
There was a problem hiding this comment.
What's the case where json_cask[:tap].to_s does not include /?
There was a problem hiding this comment.
Mainly thinking of the case where it's an empty/invalid string.
Rylan12
approved these changes
Jan 28, 2023
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 2, 2023
This is no longer needed after Homebrew/brew#14439 is merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 4, 2023
These is no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 4, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 4, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 4, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 7, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 8, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
MikeMcQuaid
added a commit
to Homebrew/formulae.brew.sh
that referenced
this pull request
Feb 15, 2023
These are no longer needed after Homebrew/brew#14439 and Homebrew/brew#14500 are merged and in a stable tag.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
HEADfor both formulae and cask JSONHEADfor the cask-source API to get the exact file on raw.githubusercontent.com rather than just whatever is newest (which is what the previous API did)Tapcorrectly when creating aCaskfrom the APIformula.jsonfile exists: print its modified time includebrew configtap.git_headas we'll be calling it a lot in the same process with the same value@Rylan12 would appreciate if you can kick the tires on this a bit. Feel free to merge when you're happy or commit to it and then merge if you want changes. Thanks ❤️ 🙇🏻