sync installed_as_dependency when changing installed_on_request#21755
sync installed_as_dependency when changing installed_on_request#21755ooye-sanket wants to merge 2 commits intoHomebrew:mainfrom
Conversation
|
@ooye-sanket thanks for the PR! I actually think the bigger change we should make is get rid of |
|
Thanks @MikeMcQuaid! I'd love to take on the bigger refactor. Before I start, could you clarify the approach?
|
Yep!
No, think we can skip/drop that too.
Can update this PR! |
|
Hi @MikeMcQuaid, I've updated the PR with the full refactor as you suggested! Here's what was done:
Could you confirm if this approach looks correct, or if there's anything I may have missed or handled incorrectly? Happy to make any adjustments! |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks great, nice work @ooye-sanket! One more tiny suggestion.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
0f98a18 to
1e52622
Compare
|
@MikeMcQuaid Added the commented-out odeprecated in cmd/list.rb as suggested. Ready for re-review! |
When marking a formula as not installed on request, also set installed_as_dependency to true, to ensure brew bundle dump correctly excludes it. Previously, brew tab --no-installed-on-request had no effect on bundle dump because the dump logic checks both Fixes Homebrew#21751
…ce of truth Replace all uses of installed_as_dependency with !installed_on_request. Stop storing installed_as_dependency in new Tabs for both formulae and casks. installed_on_request is now the single source of truth. Fixes Homebrew#21751
1e52622 to
b462b41
Compare
|
@MikeMcQuaid Rebased onto latest upstream/main, resolved all conflicts (absorbed loaded_from_internal_api additions), and added the commented-out odeprecated in cmd/list.rb as suggested. Ready for re-review! |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks! Looks good when CI is 🟢!
|
Hey @MikeMcQuaid the syntax CI check is failing, could you help point out what needs fixing? i tried a lot not getting the cause |
When marking a formula as not installed on request, also set
installed_as_dependencytotrue, to ensurebrew bundle dumpcorrectly excludes it.
Previously,
brew tab --no-installed-on-requesthad no effect onbrew bundle dumpbecause the dump logic inbrew.rbchecks both fields:Setting
installed_on_requesttofalsealone was not enough ifinstalled_as_dependencywas alsofalse— the second condition wouldstill evaluate to
true, causing the formula to be included in the dump.Fixes #21751
@MikeMcQuaid can you please review it!!
brew lgtm(style, typechecking and tests) with your changes locally?