formula: add deprecation_date and disable_date methods#10108
formula: add deprecation_date and disable_date methods#10108Rylan12 merged 2 commits intoHomebrew:masterfrom Rylan12:add-depreacte-disable-date
Conversation
|
Review period ended. |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
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.
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.
Agreed but I'll opt for the follow-up PR.
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 stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?brew manlocally and committed any changes?This PR adds the
Formula#deprecation_dateandFormula#disable_datemethods. 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.