Skip to content

dev-cmd/pr-pull: require cask/cask_loader - #23664

Merged
MikeMcQuaid merged 1 commit into
mainfrom
pr-pull-require-cask/cask_loader
Aug 26, 2026
Merged

dev-cmd/pr-pull: require cask/cask_loader#23664
MikeMcQuaid merged 1 commit into
mainfrom
pr-pull-require-cask/cask_loader

Conversation

@p-linnane

Copy link
Copy Markdown
Contributor

brew pr-pull calls Cask::CaskLoader.load when working out which packages a pull request touches, but the file only required formula, so the constant was undefined and the command failed with a NameError on cask pull requests. This was reported in https://github.com/orgs/Homebrew/discussions/7034.

Requiring cask/cask_loader fixes it, and also covers the Cask::Cask references further down the file. This is the same class of missing require as the recent cmd/deps and cmd/uses fixes. The new spec loads dev-cmd/pr-pull in a fresh Ruby process so it actually catches the missing require instead of passing because another spec already loaded the cask code.


  • 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 brew benchmark results.
  • 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?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Code (Opus 5) drafted the implementation and tests; I reviewed the diff, verified the new test fails without the fix and passes with it, and ran brew lgtm + targeted specs.


Signed-off-by: Patrick Linnane <patrick@linnane.io>
Copilot AI lite review requested due to automatic review settings August 26, 2026 17:08

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 pull request fixes a NameError in brew pr-pull for cask-related pull requests by ensuring the command file explicitly requires cask/cask_loader, so Cask::CaskLoader (and related cask constants used in the file) are always defined.

Changes:

  • Add require "cask/cask_loader" to dev-cmd/pr-pull so cask-loading code paths work reliably.
  • Add a regression spec that loads dev-cmd/pr-pull in a fresh Ruby process to catch missing requires that could be masked by prior test loads.

Reviewed changes

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

File Description
Library/Homebrew/dev-cmd/pr-pull.rb Requires cask/cask_loader so cask constants used by brew pr-pull are available.
Library/Homebrew/test/dev-cmd/pr-pull_spec.rb Adds a subprocess-based regression test to ensure dev-cmd/pr-pull loads cask support independently.

馃挕 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@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 Aug 26, 2026
Merged via the queue into main with commit f8f7616 Aug 26, 2026
50 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the pr-pull-require-cask/cask_loader branch August 26, 2026 18:19
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