-
-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
workflows: add remove-disabled-formulae workflow #67386
Conversation
This is great and all, but...
I may help with some tasks if anything. |
Good suggestions
Seems fine. I didn't start by adding it there because it felt too specific to the homebrew/core repo. No problem to add it there, though.
Why reinvent the wheel? I don't see a reason to spend time and effort creating out own when one that does the job perfectly already exists.
I see there's an action in the actions repo for this already (
Yeah, that's probably a better way to do it. I was trying to avoid making unnecessary changes but it's probably a good idea given that deprecation/disable dates are now required. |
Can take About the wheel reinventing, I just think we should use only official Actions or the ones that are written by ourselves, but if others agree with you then fine. |
The sense I have is that we don't mind using third-party actions as long as they're pinned to a specific commit. For now, I'll continue to use the third-party action but if the consensus is that having our own is helpful (or someone else writes one for Homebrew) I have no problem adjusting. In the meantime, I'll work on this some more tonight and see if I can get the other main points addressed. |
Re re-inventing the wheel: It's not just about not duplicating the work that someone else has already done. It's also about avoiding the duplication of the work that someone else is going to do. Unless Homebrew has special requirements from a create-a-pull-request action, it seems like starting from scratch also entails unnecessarily shouldering the maintenance burden of that action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless Homebrew has special requirements from a create-a-pull-request action, it seems like starting from scratch also entails unnecessarily shouldering the maintenance burden of that action.
There's also the maintenance of "updating the pinned commit" and "monitoring the security" of relying on an upstream action.
I think for something as relatively simple as creating a pull request it'd be good to have our own action that we can use amongst the many other actions we have that do so.
I don't think this needs to block this PR, though.
I've opened a PR in the brew repo in Homebrew/brew#10108 actions repo in Homebrew/actions#133. Once those are handled (in that order) this should be ready again. |
Does this enable a final review before candidate disabled formulae are removed? Or does one need to be paying attention to workflow runs to tell whether a disabled formula has been removed? I don't mind the lack of a final check; I just want to be clear about the consequences of this workflow for disabled formulae. |
No, this will simply open a PR that removes the formulae. It will still need the usual maintainer approval and all that stuff. Similar to the |
Ah, I was looking at your two other PRs which lead me to the impression that it would silently delete disabled formulae. I should've been looking at this one. 😅 |
I tested this here and it seems to work as expected. I'm going to merge and we'll see how it goes! |
Success! 🎉🥳 |
brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingbrew install <formula>
)?This PR adds a workflow that will automatically remove formulae that have a disable date of more than one year ago. See discussion in #66360. I haven't really tested the actual PR-creation part of this (although I've tested the old-formula-detection part locally), so if it seems okay I'd suggest merging and seeing what happens.