-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
formula: add deprecation_date and disable_date methods #10108
Conversation
Review period ended. |
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.
I'm fine with this but I wonder if as-well/instead it'd be easier for the actions to have this be part of the JSON output? I'd imagine @EricFromCanada would perhaps also use it for formulae.brew.sh. No worries if not (it's not blocking).
# Returns `nil` if no date is specified. | ||
# @!method deprecation_date | ||
# @return Date | ||
delegate deprecation_date: :"self.class" |
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.
Might be nice to have a delegate ... :"self.class"
helper method because it's used in a tonne of places (as a follow-up PR).
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.
Agreed but I'll opt for the follow-up PR.
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.
Yup, thanks!
How would one get JSON information for all formulae in a ruby script? I know that Regardless, I think it makes sense to have the deprecation/disable date and reason in the JSON output, so I'll add it. That was Eric and others can use it if they desire. |
I was thinking you might prefer shelling out to |
Is there a significant speed/other benefit to doing that? We're already working in ruby so shelling out just to bring it back into ruby doesn't seem that helpful to me. Plus, speed isn't going to be an issue here. We only need the action to run once a day anyway so if it takes five minutes to run it's not a huge deal. (I totally get that you're not pushing it I'm just curious why it might be better) |
Nope, thought you might be shelling out from JS. |
Ah, gotcha! In that case, makes a lot of sense. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?brew man
locally and committed any changes?This PR adds the
Formula#deprecation_date
andFormula#disable_date
methods. These will return the respective dates when they exist. If not, the methods returnnil
. This is needed for Homebrew/actions#133 and Homebrew/homebrew-core#67386.I'm marking this as critical because it is blocking those other PRs. Since the PR is a simple change that only adds methods that aren't used in the codebase yet, I don't feel the full 24 hours are needed. However, I will wait to merge until I'm ready to move forward with the other PRs.