Skip to content

Remove unused instance variables in API code - #23346

Merged
MikeMcQuaid merged 1 commit into
mainfrom
remove-unused-api-ivars
Jul 29, 2026
Merged

Remove unused instance variables in API code#23346
MikeMcQuaid merged 1 commit into
mainfrom
remove-unused-api-ivars

Conversation

@dduugg

@dduugg dduugg commented Jul 29, 2026

Copy link
Copy Markdown
Member

Removes instance-variable assignments that are never read.

  • In Homebrew::API::JSONDownloadStrategy and Homebrew::API::JSONDownload, @stale_seconds and @target are assigned in initialize but never read (not in these classes, nor in the AbstractDownloadStrategy parent or Downloadable mixin). The value is used through meta[:target]/meta[:stale_seconds] and the target/stale_seconds arguments instead. With both ivars gone, JSONDownloadStrategy#initialize only called super, so it is removed too.
  • APIHashable#@old_homebrew_cellar is saved in generating_hash! but never restored anywhere (only @old_homebrew_prefix, @old_home and @old_git_config_global are restored, and HOMEBREW_CELLAR is not monkeypatched in the first place).

Instance-variable assignments like these aren't reported by brew deadcode (Spoom models methods and constants, not ivars), which is why they lingered. Verified with git grep that nothing reads them.


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Claude (Claude Code) found the initial write-only instance variables (@stale_seconds, @old_homebrew_cellar) and drafted the removal. The @target removal was prompted by Copilot's review. I verified with git grep that nothing reads them and ran brew lgtm (style, typecheck, tests) successfully.


@dduugg
dduugg marked this pull request as ready for review July 29, 2026 01:09
Copilot AI review requested due to automatic review settings July 29, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 removes write-only instance-variable assignments in Homebrew’s API-related code to reduce dead state and keep the API implementation easier to reason about.

Changes:

  • Removes unused @stale_seconds assignments from Homebrew::API::JSONDownloadStrategy and Homebrew::API::JSONDownload.
  • Removes unused @old_homebrew_cellar capture from APIHashable#generating_hash!.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Library/Homebrew/api/json_download.rb Drops unused @stale_seconds ivars in JSON API download strategy/helper classes.
Library/Homebrew/api_hashable.rb Removes unused @old_homebrew_cellar assignment during API hash generation monkeypatch setup.
Comments suppressed due to low confidence (1)

Library/Homebrew/api/json_download.rb:42

  • @target is assigned but never read (this class delegates the target to the URL specs/downloader). Keeping an unused instance variable here is inconsistent with the PR’s goal of removing write-only ivars.
      def initialize(url, target:, stale_seconds:)
        super()
        @url = T.let(URL.new(url, using: API::JSONDownloadStrategy, target:, stale_seconds:), URL)
        @target = target
      end

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/api/json_download.rb Outdated
These instance variables are assigned but never read:

- `@stale_seconds` and `@target` in `Homebrew::API::JSONDownloadStrategy`
  and `Homebrew::API::JSONDownload`; the value is used via `meta[:target]`/
  `meta[:stale_seconds]` and the `target`/`stale_seconds` arguments, not the
  ivars. (`JSONDownloadStrategy#initialize` then only called `super`, so drop
  it too.)
- `@old_homebrew_cellar` in `APIHashable`, which is saved during API
  generation but never restored (only the prefix, home and git config are).

Claude-Session: https://claude.ai/code/session_011BgWRBydtdkvt8ocqH4nj9
@dduugg
dduugg force-pushed the remove-unused-api-ivars branch from f71fb6f to 7e2cd50 Compare July 29, 2026 04:45

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 1679a55 Jul 29, 2026
43 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the remove-unused-api-ivars branch July 29, 2026 07:56
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.

3 participants