Skip to content

api/formula/formula_struct_generator: defer placeholder replacement - #23780

Merged
MikeMcQuaid merged 1 commit into
mainfrom
api-placeholders
Sep 4, 2026
Merged

api/formula/formula_struct_generator: defer placeholder replacement#23780
MikeMcQuaid merged 1 commit into
mainfrom
api-placeholders

Conversation

@cho-m

@cho-m cho-m commented Sep 4, 2026

Copy link
Copy Markdown
Member

This method runs FormulaStruct.from_hash at the end which will call ::Formula.deep_remove_placeholders. The latter detects if generating JSON so that placeholders are preserved until loaded by user.

def self.from_hash(formula_hash)
formula_hash = ::Formula.deep_remove_placeholders(formula_hash)

def deep_remove_placeholders(value)
return value if generating_hash?


  • 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.

This method runs `FormulaStruct.from_hash` at the end which will call
`::Formula.deep_remove_placeholders`. The latter detects if generating
JSON so that placeholders are preserved until loaded by user.
Copilot AI balanced review requested due to automatic review settings September 4, 2026 16:46

@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!

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.

馃煝 Approval recommended

The delegated behavior matches FormulaStruct.from_hash semantics and is adequately tested.

Pull request overview

Defers placeholder replacement to FormulaStruct.from_hash, preserving placeholders during API JSON generation.

Changes:

  • Removes eager caveat placeholder replacement.
  • Adds coverage for recursive replacement and JSON-generation preservation.
  • Consolidates generator tests around shared setup.
File summaries
File Description
Library/Homebrew/api/formula/formula_struct_generator.rb Delegates placeholder handling to FormulaStruct.
Library/Homebrew/test/api/formula/formula_struct_generator_spec.rb Tests placeholder behavior and refactors setup.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

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

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 68a24a0 Sep 4, 2026
59 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the api-placeholders branch September 4, 2026 17:18
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