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

Reorganize module hierarchy #738

Closed
wants to merge 30 commits into from
Closed

Reorganize module hierarchy #738

wants to merge 30 commits into from

Conversation

kozross
Copy link
Contributor

@kozross kozross commented Nov 4, 2024

This is the first attempt to try and make some sense of the module hierarchy. The overall goals were:

  1. Users should need to only import Plutarch.Prelude and get everything they need
  2. Plutarch itself shouldn't import Plutarch.Prelude, only things in the Plutarch.Internal.* namespace
  3. plutarch-ledger-api and plutarch-testlib should rely minimally on imports from Plutarch.Internal.*

Additionally, there were a couple of changes:

  1. All builtins should have a pbuiltinMyBuiltinName in Plutarch.Internal.Builtin, allowing them to be called without having to wonder what their signatures are.
  2. Plutarch.* has been significantly emptied.
  3. Plutarch.Internal.Builtin contains (most of) the types that we have to have because they're mandated by Plutus.
  4. Some modules got merged.

This is not the final outcome, I just wanted to get something reviewed and merged, or the diffs would get too huge.

@kozross kozross requested a review from SeungheonOh November 4, 2024 20:35
@@ -0,0 +1,810 @@
{-# LANGUAGE FlexibleInstances #-}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is quite massive. More so than I thought, would it be better to make Plutarch.Builtins.XYZ modules instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree.

@SeungheonOh
Copy link
Collaborator

Will also resolve #735

@kozross kozross closed this Nov 20, 2024
@SeungheonOh
Copy link
Collaborator

Notes: this is closed because of nasty conflict

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.

2 participants