-
Notifications
You must be signed in to change notification settings - Fork 15
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 error type and messages in Cardano.CLI.EraBased.Run
#156
Conversation
Cardano.CLI.EraBased.Run
Cardano.CLI.EraBased.Run
4d716d6
to
afa82b9
Compare
afa82b9
to
d943a09
Compare
Cardano.CLI.EraBased.Run
Cardano.CLI.EraBased.Run
ced4540
to
26e6a5d
Compare
runEraBasedCommand = \case | ||
EraBasedGovernanceCmds cmd -> runEraBasedGovernanceCmds cmd | ||
|
||
runEraBasedGovernanceCmds :: () | ||
=> EraBasedGovernanceCmds era | ||
-> ExceptT () IO () | ||
-> ExceptT AnyEraCmdError IO () |
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.
Are the error types going to follow the command structure or are we doing one large sum type for all our errors?
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.
But I think we should not create a distinction between era-based command errors and legacy errors. We can actually share the same types, and in fact it's preferably because it means less changes when we move legacy stuff into era-based.
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 reckon we should call this type CmdError
and put it in Cardano.CLI.EraBased.Error
. Then the legacy code can use this type instead later instead of LegacyClientCmdError
.
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.
We already have ClientCommandErrors
in Cardano.CLI.Run. Should we reuse this one as a top-level umbrella error type?
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.
Not yet. It includes lots of legacy stuff as members.
26e6a5d
to
3c9a5d5
Compare
3c9a5d5
to
780d376
Compare
Changelog
Context
n/a
Checklist
See Running tests for more details
.cabal
files are updatedhlint
. See.github/workflows/check-hlint.yml
to get thehlint
versionstylish-haskell
. See.github/workflows/stylish-haskell.yml
to get thestylish-haskell
versionghc-8.10.7
andghc-9.2.7