Skip to content

Conversation

hyuraku
Copy link
Contributor

@hyuraku hyuraku commented Oct 11, 2025

…ed_as_dependency and installed_on_request

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

solve the comment # TODO: change this to always return a boolean

@hyuraku hyuraku force-pushed the tab-install-status-type-safety branch from e76825a to 8d506bc Compare October 12, 2025 09:52
@hyuraku hyuraku force-pushed the tab-install-status-type-safety branch from 7463539 to 542b11b Compare October 13, 2025 12:36
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks! A few tweaks.

@Copilot Copilot AI review requested due to automatic review settings October 16, 2025 12:53
@hyuraku hyuraku force-pushed the tab-install-status-type-safety branch from 542b11b to eb6ee32 Compare October 16, 2025 12:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the installed_as_dependency and installed_on_request attributes in the Tab class to always return boolean values instead of nullable booleans. The change introduces new tracking methods to distinguish between explicitly set false values and unset/missing values.

  • Added installed_on_request_present? and installed_as_dependency_present? methods to track whether these attributes were explicitly set
  • Updated type signatures from T.nilable(T::Boolean) to T::Boolean with default values of false
  • Modified the autoremove logic to check for presence before evaluating the boolean value

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Library/Homebrew/tab.rb Added presence tracking for dependency/request attributes, updated type signatures to non-nullable booleans, and added new predicate methods
Library/Homebrew/utils/autoremove.rb Updated unused formula detection to check for attribute presence before evaluating boolean values
Library/Homebrew/test/utils/autoremove_spec.rb Updated test mocks to include the new installed_on_request_present? method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks @hyuraku!

@MikeMcQuaid MikeMcQuaid enabled auto-merge October 16, 2025 13:29
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Oct 21, 2025
Merged via the queue into Homebrew:main with commit 61d4238 Oct 21, 2025
74 of 77 checks passed
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.

4 participants