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: rewrite generation as a Homebrew command based on the API #45

Merged
merged 1 commit into from
May 28, 2024

Conversation

SMillerDev
Copy link
Member

@SMillerDev SMillerDev commented May 21, 2024

This recreates the CI command as a Homebrew command that's based on the cask API that users also use. This means that it'll be slightly slower to update than it is now, but also more stable.

TODO:

  • Let the command make commits
  • Allow the command to cleanup files that no longer exist
  • Add disabled/deprecated status

@SMillerDev SMillerDev force-pushed the feat/ci/use_homebrew_cmd branch 3 times, most recently from d78a05d to cd25e97 Compare May 21, 2024 10:23
@tani
Copy link
Member

tani commented May 21, 2024

Thank you for the contribution. It seems that it's smart.
I review the request now. Thank you again.

@SMillerDev SMillerDev force-pushed the feat/ci/use_homebrew_cmd branch 5 times, most recently from 1d23589 to 93ae0d4 Compare May 26, 2024 10:17
@SMillerDev SMillerDev marked this pull request as ready for review May 26, 2024 10:17
@SMillerDev SMillerDev force-pushed the feat/ci/use_homebrew_cmd branch 2 times, most recently from 71ead20 to b511a57 Compare May 26, 2024 10:26
@SMillerDev
Copy link
Member Author

@MikeMcQuaid made the style check continue on error since the generated formulae don't all conform to the style check. Missing tests and unaligned array keys are the big factors there.

Otherwise it's ready to go for another review.

@MikeMcQuaid
Copy link
Member

made the style check continue on error since the generated formulae don't all conform to the style check.

Remove the style check rather than wasting time running it and then ignoring the result.

Also, please open an issue for this to be fixed, we shouldn't be generating invalid formulae in an official tap.

@SMillerDev
Copy link
Member Author

Also, please open an issue for this to be fixed, we shouldn't be generating invalid formulae in an official tap.

Some can be corrected, some my code fixes, but some are simply because casks don't have as strict a ruleset as formula do.

I'll make an issue with a summary of the issues after this PR.

@SMillerDev SMillerDev mentioned this pull request May 27, 2024
6 tasks
@SMillerDev SMillerDev force-pushed the feat/ci/use_homebrew_cmd branch 2 times, most recently from d8755c5 to fe45f5f Compare May 27, 2024 07:53
@SMillerDev
Copy link
Member Author

Style issue is created. If anyone has an idea how to write tests for these formula we can fix that part already.

@MikeMcQuaid
Copy link
Member

If anyone has an idea how to write tests for these formula we can fix that part already.

I think either:

  • use rubocop:disable rules
  • make it in brew style or global RuboCop rules that they aren't required here
  • something like test do \n true \n end

cmd/generate-linux-fonts.rb Outdated Show resolved Hide resolved
@SMillerDev
Copy link
Member Author

2238 files inspected, 347 offenses detected, 6 offenses autocorrectable

after these last changes.

@SMillerDev SMillerDev force-pushed the feat/ci/use_homebrew_cmd branch 2 times, most recently from 32590b2 to b43f05e Compare May 28, 2024 08:23
@SMillerDev
Copy link
Member Author

@MikeMcQuaid I can't merge here so if you're okay with it, go ahead!

@MikeMcQuaid MikeMcQuaid merged commit c6c12ea into Homebrew:master May 28, 2024
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants