Skip to content
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: handle nil URLs #14803

Merged
merged 1 commit into from Feb 24, 2023
Merged

cask: handle nil URLs #14803

merged 1 commit into from Feb 24, 2023

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Feb 24, 2023

This generally shouldn't happen but it's probably acceptable for casks under the minimum macOS.

There are a few cases I see this happen but for above the minimum macOS. The fix here still covers them, but I'll open a change to readall to catch that there as I don't think the cask should be called valid in those scenarios.

@Bo98 Bo98 added the critical Critical change which should be shipped as soon as possible. label Feb 24, 2023
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

@Bo98
Copy link
Member Author

Bo98 commented Feb 24, 2023

(This fixes formulae.brew.sh generation)

@Bo98 Bo98 enabled auto-merge February 24, 2023 18:14
@Bo98 Bo98 merged commit ba8fd26 into Homebrew:master Feb 24, 2023
@Bo98 Bo98 deleted the cask-nil-url branch February 24, 2023 18:34
@MikeMcQuaid
Copy link
Member

Thanks @Bo98!

Should probably use brew generate-*-api commands in Homebrew/homebrew-core and Homebrew/homebrew-cask jobs as required steps to avoid breaking API generation with a cask merge.

@Bo98
Copy link
Member Author

Bo98 commented Feb 27, 2023

It was actually a brew change that caused it.

For homebrew-core/cask: that makes sense. Though I'm of the belief that brew readall should cover it and if it doesn't then we've got poor validation (hence the change I proposed to that).

@MikeMcQuaid
Copy link
Member

It was actually a brew change that caused it.

@Bo98 same deal then, I guess: we should make this commands part of the CI workflow so we can't break them.

Though I'm of the belief that brew readall should cover it and if it doesn't then we've got poor validation (hence the change I proposed to that).

It should but I think it's not safe for us to rely on that. There's many other ways this can get broken that readall might not pick up.

@github-actions github-actions bot added the outdated PR was locked due to age label Mar 31, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants