Skip to content

Conversation

botantony
Copy link
Member

@botantony botantony commented Oct 12, 2025

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

Draft PR that implements pypi_packages DSL support. Maybe creating a new class is an overkill, but I do not want to use simple Hash for better types

TODO: add tests and documentation

Closes #20832

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.

Looks good so far!

@botantony botantony changed the title [WIP] feat: add pypi_packages formula DSL feat: add pypi_packages formula DSL Oct 15, 2025
@botantony botantony marked this pull request as ready for review October 15, 2025 11:57
@Copilot Copilot AI review requested due to automatic review settings October 15, 2025 11:57
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

Adds a new pypi_packages DSL to formulas to declaratively specify PyPI package mapping (primary package name, extras, exclusions, dependencies, or manual-update flag) and refactors resource update logic to use a new typed helper class.

  • Introduces PypiPackages helper class with typed accessors
  • Adds Formula#pypi_packages DSL plus integration in update_python_resources!
  • Updates AST constants and adds initial test coverage for PypiPackages.from_json_file

Reviewed Changes

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

Show a summary per file
File Description
utils/pypi.rb Refactors PyPI resource update logic to use PypiPackages instance instead of raw tap JSON
pypi_packages.rb Adds new typed helper class encapsulating PyPI mapping data
formula.rb Adds DSL method, attribute, and documentation for pypi_packages
ast_constants.rb Registers pypi_packages in allowable AST method calls
test/pypi_packages_spec.rb Adds tests for constructing PypiPackages from tap JSON

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!

@botantony botantony marked this pull request as draft October 15, 2025 17:48
@botantony
Copy link
Member Author

Converting this to draft to prevent merging. Sorry for noticing it too late, but for some reason delegate pypi_packages: :"self.class" does not work. If this method is used in a formula, it throws an error

Error: <owner>/<tap>/<formula>: undefined method 'pypi_packages' for class Formulary::FormulaNamespace<namespace>::<Formula>

@botantony botantony marked this pull request as ready for review October 15, 2025 19:58
@botantony
Copy link
Member Author

Well, the problem was way more trivial than I thought. Need to pay more attention to what I'm doing next time 😅

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.

Replace pypi_formula_mappings.json with a new DSL

2 participants