-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add module for UTxO type and operations #710
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @Jimbo4350 thoughts?
0ce0fcf
to
a2dbc0a
Compare
a07f563
to
1e931e1
Compare
1e931e1
to
1235545
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @Jimbo4350 thoughts?
@@ -248,6 +250,7 @@ library | |||
Cardano.Api.Ledger | |||
Cardano.Api.Network | |||
Cardano.Api.Shelley | |||
Cardano.Api.UTxO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep this module hidden and expose the functionality from Cardano.Api.
We need to do an audit on how we expose things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. This module by design is intended to be imported qualified, similar to container modules such as Data.Map
, as it contains function names which commonly conflict.
@@ -0,0 +1,3 @@ | |||
module Cardano.Api.UTxO (module Cardano.Api.Query.UTxO) where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These modules exposing modules generally are annoying. See my comment above as to how this functionality should be exposed for the time being.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also see above.
1235545
to
f6a3ba2
Compare
Changelog
Context
Additional context for the PR goes here. If the PR fixes a particular issue please provide a link to the issue.
How to trust this PR
Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.
Checklist