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

feat: check if casks should warn about rosetta #17573

Merged

Conversation

SMillerDev
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This should warn in cases like Homebrew/homebrew-cask#177913, so that authors can add the required caveats.

@SMillerDev SMillerDev force-pushed the feat/audit/check_cask_binary_rosetta branch from e681f60 to d8fbcda Compare June 27, 2024 11:59
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few optional tweaks.

Library/Homebrew/cask/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/cask/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/cask/audit.rb Outdated Show resolved Hide resolved
@SMillerDev SMillerDev force-pushed the feat/audit/check_cask_binary_rosetta branch 2 times, most recently from 48951aa to 4a36b31 Compare June 27, 2024 12:09
@SMillerDev
Copy link
Member Author

Clearly I haven't been a mac user long enough, since I didn't mention Rosetta 2 😄.

Did another small addition, and included your changes @MikeMcQuaid

@bevanjkay
Copy link
Member

Thanks @SMillerDev

@SMillerDev SMillerDev force-pushed the feat/audit/check_cask_binary_rosetta branch from 4a36b31 to 8dffc31 Compare June 27, 2024 12:18
@SMillerDev SMillerDev force-pushed the feat/audit/check_cask_binary_rosetta branch from 8dffc31 to 5fc3af1 Compare June 27, 2024 12:22
@SMillerDev SMillerDev merged commit fc3ba1b into Homebrew:master Jun 27, 2024
25 checks passed
@wickles
Copy link
Contributor

wickles commented Jun 27, 2024

This does not cover the 32-bit intel case from the referenced PR.

❯ brew audit --cask --online nightingale
==> Downloading and extracting artifacts
...
audit for nightingale: failed
 - Artifacts require Rosetta 2 but this is not indicated by the caveats!
nightingale
  * line 5, col 2: Artifacts require Rosetta 2 but this is not indicated by the caveats!
Error: 1 problem in 1 cask detected.

In this case rosetta caveat should not be added and another solution is needed instead as mentioned in the other PR.

@SMillerDev
Copy link
Member Author

This does not cover the 32-bit intel case from the referenced PR.

Those should be removed:

Starting with macOS Catalina, 32-bit apps are no longer compatible with macOS.

https://support.apple.com/en-us/103076

@wickles
Copy link
Contributor

wickles commented Jun 27, 2024

Those should be removed

Removed, disabled, or deprecated? pgadmin3 in particular still gets lots of downloads (243 in 30 days).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants