-
Notifications
You must be signed in to change notification settings - Fork 722
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
Combinators for TxBodyContent and related types #4941
Conversation
40a42bb
to
f85ef25
Compare
f85ef25
to
09da340
Compare
4b3c24a
to
7f64edf
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.
Left my 2 lovelaces as a user of this library.
Just a minor note. No new types or classes are introduced in this PR, just new instances 😉 |
424ea6a
to
16e60df
Compare
7a1ddcc
to
c6dd515
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.
Nice work 👍 . We definitely aren't interested in building ViewTx
transactions, so you can remove support for that.
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.
I like it and it's very much what we currently do in the upstream cardano-api. So hydra will be using this 👍
9c643dd
to
2bef436
Compare
3cf60ab
to
fadb264
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! Just a couple naming suggestions.
fadb264
to
fef1db7
Compare
TxBodyContent has a function that constructs one of two default values for
BuiltTx
andViewTx
.This is an alternative to #4458.
We don't seem to be able to agree on what the
Semigroup
instance should do out of a number alternatives and if there is confusion with choosing the behaviour, there will likely be confusion using the instance.The idea behind this alternative is that we shouldn't arbitrarily choose a behaviour for the
Semigroup
instance because there is a high probability people who use our instances might assume it does something else.This works with both
ViewTx
andBuildTx
.