-
Notifications
You must be signed in to change notification settings - Fork 721
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 toEraInMode
for conway
#5175
Conversation
79a57d2
to
63bdf61
Compare
toEraInMode ByronEra CardanoMode = Just ByronEraInCardanoMode | ||
toEraInMode ShelleyEra CardanoMode = Just ShelleyEraInCardanoMode | ||
toEraInMode AllegraEra CardanoMode = Just AllegraEraInCardanoMode | ||
toEraInMode MaryEra CardanoMode = Just MaryEraInCardanoMode | ||
toEraInMode AlonzoEra CardanoMode = Just AlonzoEraInCardanoMode | ||
toEraInMode BabbageEra CardanoMode = Just BabbageEraInCardanoMode | ||
toEraInMode _ _ = Nothing | ||
toEraInMode ConwayEra CardanoMode = Just ConwayEraInCardanoMode |
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'm wondering if it would make sense to add a property test to catch similar bugs in the future.
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.
The type system should be able to catch this. By removing _
, not adding new eras in the pattern match will make it in-exhaustive so we'll get an error.
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.
Assuming that the mapping is correct in the first place 😃
63bdf61
to
a8fcbed
Compare
a8fcbed
to
04d2497
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.
Yeah, avoiding catch all _
in this kind of functions is a really good practice!
👍
f64645e
to
f8158fe
Compare
f8158fe
to
d9f1032
Compare
Description
The pattern match for
ConwayEra
era was missing and is added.Also restructure the pattern match so this doesn't happen again for future eras.
Checklist
See Runnings tests for more details
CHANGELOG.md
for affected package.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
Note on CI
If your PR is from a fork, the necessary CI jobs won't trigger automatically for security reasons.
You will need to get someone with write privileges. Please contact IOG node developers to do this
for you.