-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add auto_update and app_name column to homebrew_packages table
#8520
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
Add auto_update and app_name column to homebrew_packages table
#8520
Conversation
c0bd4cb to
a1a1610
Compare
auto_update column to homebrew_packages table
directionless
left a comment
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.
FWIW I'd generally prefer to see the two tables updated in different PRs
a1a1610 to
bb12da7
Compare
I removed the commits regarding #8502 as they were not adding anything useful to this PR. |
auto_update column to homebrew_packages tableauto_update and app_name column to homebrew_packages table
|
@directionless : I refactored a bit, in order to be able to add the |
directionless
left a comment
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.
This is adding a lot of complexity, but maybe that's okay
@directionless: I can go back to the drawing board if y'all think it's too complex. Happy to chat about it |
|
/easycla |
|
Regarding this PR, I'm honestly wondering if this would be worth creating a brand new table specifically for homebrew casks, and slowly add as much details as we could, given that the homebrew "formulas" are much more static, if that makes sense. |
|
Apologies for the delay here. We discussed this in the last office hours and think the approach of updating the existing table makes sense. I can give this a review to help move it forward. |
zwass
left a comment
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.
IMO it's ready to merge after updating the foreach. Thanks!
2ba840d to
a020dc9
Compare
It should be good now @zwass, thanks for taking the time to review my code. |
zwass
left a comment
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.
Looks good, thank you!
Implementation of option 5.1 from blueprint #8476
It adds a new
auto_updatescolumn, that will be0or1for casks, and empty for formulas.It also adds an
app_namecolumn that will provide the name of the installed app (usually linked in the Application folder)