-
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
Fix missing script proposals in transaction building #602
Fix missing script proposals in transaction building #602
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!
15dfd1b
to
3ee255f
Compare
b98416b
to
d34b151
Compare
0a9ace3
to
6e1b01d
Compare
64743e2
to
200b65c
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.
Revert all the changes wrt to data types. I don't want to break the api twice because there may be a way to remove BuildTxWith
. Just fix the bug we have encountered.
9246b94
to
ce1f6d9
Compare
ce1f6d9
to
91289ff
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, some instances can be removed and some minor refactors.
cardano-api/internal/Cardano/Api/Governance/Actions/ProposalProcedure.hs
Outdated
Show resolved
Hide resolved
cardano-api/internal/Cardano/Api/Governance/Actions/VotingProcedure.hs
Outdated
Show resolved
Hide resolved
cardano-api/test/cardano-api-test/Test/Cardano/Api/Typed/TxBody.hs
Outdated
Show resolved
Hide resolved
ed6a214
to
f170034
Compare
f170034
to
55fadc9
Compare
@@ -800,7 +815,7 @@ genMaybeFeaturedInEra | |||
-> f (Maybe (Featured eon era a)) | |||
genMaybeFeaturedInEra f = | |||
inEonForEra (pure Nothing) $ \w -> | |||
pure Nothing <|> fmap Just (genFeaturedInEra w (f w)) | |||
Just <$> genFeaturedInEra w (f w) |
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.
Why? We also want to generate Nothing
even when the feature is available in a given era.
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.
Previous implementation was always generating Nothing
, because the first argument of <|>
was never empty
.
Changelog
Context
Fixes #594
Checklist