-
Notifications
You must be signed in to change notification settings - Fork 156
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
Change applySTS to return NonEmpty (PredicateFailure s) #4086
Conversation
1c2794b
to
d551071
Compare
The ghc 8.10.7 builds are failing because there's no I could fix it by using All the uses of |
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.
Aside from required changes to versions and changelogs this PR looks great.
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Examples/Consensus.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-test/src/Test/Cardano/Ledger/Constrained/Preds/Tx.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/test/Test/Cardano/Ledger/Shelley/RulesTests.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/test/Test/Cardano/Ledger/Shelley/Examples/Mir.hs
Outdated
Show resolved
Hide resolved
d551071
to
4223cd4
Compare
Rebased on |
4223cd4
to
2a2d53a
Compare
I rebased, but kept the review changes as separate commits. I'll squash before merging. |
2a2d53a
to
7f09bcc
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.
Beautiful!!
Thank you.
Closes #4054
Description
This eliminates the anomalous possibility of a
Left
value containing no failures, which we can't treat as a success because it doesn't contain a return value.I wondered about adding a type alias for
NonEmpty (PredicateFailure f)
or evenEither (NonEmpty (PredicateFailure f)) r
to simplify the function type signatures, but decided that would make the code less clear for someone not familiar with the alias.This touched a lot more lines than I was expecting.
Checklist
.cabal
andCHANGELOG.md
files according to theversioning process.
.cabal
files for all affected packages are updated. If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)CHANGELOG.md
for the affected packages. New section is never added with the code changes. (See RELEASING.md)fourmolu
(usescripts/fourmolize.sh
)scripts/cabal-format.sh
)hie.yaml
has been updated (usescripts/gen-hie.sh
)