-
Notifications
You must be signed in to change notification settings - Fork 122
LocalDateDeserializer
should consider coercionConfig settings
#240
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
…il for Numbers' in coerceConfig
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.
Looks good to me, we can look for @cowtowncoder for a final review. Thanks for the PR!
@@ -476,7 +478,7 @@ public void testLenientDeserializeFromNumberInt() throws Exception { | |||
mapper.configOverride(LocalDate.class) | |||
.setFormat(JsonFormat.Value.forShape(JsonFormat.Shape.NUMBER_INT)); | |||
|
|||
assertEquals("The value is not correct.", LocalDate.of(1970, Month.MAY, 04), | |||
assertEquals("The value is not correct.", LocalDate.of(1970, Month.MAY, 4), |
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.
Odd, I wonder why it used to be 04 vs. 4.. anyway, this cleanup looks fine to me.
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.
Maybe seemed more readable by original author? (I didn't write it)
Looks good to me. Thank @maciekdeb ! Only one more thing I need before merging: if you have not sent a CLA before (I saw one with similar name but not bound to this github handle?), I'd need one from here: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf and the usual way is to print it, fill & sign, scan/photo, email to Thank you once again for the contribution! |
Thanks @kupci & @cowtowncoder. I've just sent the CLA to this email address. I guess other date classes also misses it, when I have more time I can review them, I changed only LocalDate for now as this was the one I stumbled upon. |
LocalDateDeserializer
should consider coercionConfig settings
@maciekdeb Thank you again! Yes, I think all others could and should be retrofitted too -- that would be valuable contribution. Same also goes for |
Currently LocalDateDeserializer does not take into account the following configuration:
setCoercion(CoercionInputShape.Integer, CoercionAction.Fail);