Skip to content
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

Revamp installed_on_request handling #18768

Merged
merged 1 commit into from
Nov 20, 2024
Merged

Conversation

MikeMcQuaid
Copy link
Member

  • reinstall and upgrade no longer mark as installed on request, with or without names specified, but preserve the version from the tab instead
  • default install_on_request to false rather than true
  • only set installed in request in a tab if it's missing rather than false

@cho-m feel free to push any changes/test fixes directly to this PR as desired

Fixes #18754

@MikeMcQuaid MikeMcQuaid requested a review from cho-m November 13, 2024 17:31
@MikeMcQuaid MikeMcQuaid force-pushed the revamp_installed_on_request branch from 57c41bb to 49eba46 Compare November 14, 2024 15:00
Copy link
Member

@cho-m cho-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay with changes avoiding excess installed-on-request (given we often recommend brew [reinstall | upgrade] ... as a way of fixing some issues)

I added comments on behavior differences that may need to be verified as intentional.

Library/Homebrew/cmd/reinstall.rb Show resolved Hide resolved
Library/Homebrew/install.rb Outdated Show resolved Hide resolved
Library/Homebrew/upgrade.rb Outdated Show resolved Hide resolved
@MikeMcQuaid MikeMcQuaid force-pushed the revamp_installed_on_request branch from 49eba46 to 319cc41 Compare November 15, 2024 09:34
@MikeMcQuaid MikeMcQuaid requested a review from cho-m November 15, 2024 09:35
Library/Homebrew/upgrade.rb Outdated Show resolved Hide resolved
@cho-m
Copy link
Member

cho-m commented Nov 19, 2024

Otherwise LGTM. No problems with merging after above fix.

- `reinstall` and `upgrade` no longer mark as installed on request,
  with or without names specified, but preserve the version from the
  tab instead
- default `install_on_request` to `false` rather than `true`
- only set installed in request in a tab if it's missing rather than
  false

Co-authored-by: Michael Cho <[email protected]>
@MikeMcQuaid MikeMcQuaid force-pushed the revamp_installed_on_request branch from 5f8d841 to 4d4531c Compare November 19, 2024 08:40
@MikeMcQuaid MikeMcQuaid merged commit d8242da into master Nov 20, 2024
28 checks passed
@MikeMcQuaid MikeMcQuaid deleted the revamp_installed_on_request branch November 20, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formula dependency incorrectly marked as installed_on_request upon upgrade/migration
2 participants