Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move builder_util into builder itself #1637

Closed
Tracked by #1636
anshumanmohan opened this issue Jul 25, 2023 · 0 comments · Fixed by #1665
Closed
Tracked by #1636

Move builder_util into builder itself #1637

anshumanmohan opened this issue Jul 25, 2023 · 0 comments · Fixed by #1665
Labels
C: calyx-py Items that have to do with the builder library

Comments

@anshumanmohan
Copy link
Contributor

anshumanmohan commented Jul 25, 2023

Quoting Adrian from #1630:

I have an idea for how to make this API even more pleasant to use. Instead of top-level functions that take a ComponentBuilder as an argument, we could wrap the underlying ComponentBuilder (one way or another) and provide these as methods. The methods could then have shorter names too. So using the API would look more like comp.eq(a, b) instead of insert_eq(comp, a, b).

The options include:

  1. Move all of these into ComponentBuilder. Easy peasy, if not exactly organized.
  2. Subclass ComponentBuilder with a new class that adds these methods, if you want them.
  3. A new wrapper class, ComponentHelper, has ComponentBuilder as a member. Then all these methods just access self.comp to do their work.

My gut feeling says 1, 3, and then 2, in that order. That is, it doesn't seem like a great sin to just put these right in ComponentBuilder. The whole idea there is to make life more convenient when using this builder-DSL, so the more convenience methods the better.

@anshumanmohan anshumanmohan changed the title Move builder_util into builder itself. Move builder_util into builder itself Jul 25, 2023
@anshumanmohan anshumanmohan added the C: calyx-py Items that have to do with the builder library label Jul 27, 2023
@anshumanmohan anshumanmohan linked a pull request Aug 15, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: calyx-py Items that have to do with the builder library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant