-
-
Notifications
You must be signed in to change notification settings - Fork 227
Handle Unset Enums when deserializing #306
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #306 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 47 47
Lines 1385 1385
=========================================
Hits 1385 1385 Continue to review full report at Codecov.
|
CC @dbanty. Good chance we have this issue other places as well but |
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! Thanks as always @bowenwr 😄
Unrelated, but this makes me think that maybe we should look into hypothesis - I've never used it personally, but I think it would help us in covering edge cases like this that slip by unnoticed.
Thanks for a speedy review @emann (didn't there used to be a guitar in there?)! I don't have personal experience with hypothesis, but agree that property-based testing might help us catch a lot more. |
Thanks @emann! Anything blocking this from a merge? |
@bowenwr nope, sorry! I've enabled auto-merge, if you could add a changelog entry and pull in the latest changes from main that'd be swell and it should merge automatically |
9db693b
to
f1b3e4e
Compare
@emann Done, thank you! |
When an
Enum
isUnset
during deserialization, it causes an error by attempting to instantiate it fromUNSET
.