-
Notifications
You must be signed in to change notification settings - Fork 174
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
Create Builder for KotlinModule #306
Conversation
@@ -14,16 +14,22 @@ class KotlinModule constructor ( | |||
val reflectionCacheSize: Int = 512, | |||
val nullToEmptyCollection: Boolean = false, | |||
val nullToEmptyMap: Boolean = false, | |||
val nullisSameAsDefault: Boolean = false | |||
val nullIsSameAsDefault: Boolean = false |
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 fixed the camelCasing on this property.
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.
good, thanks.
import org.junit.Assert.* | ||
import org.junit.Test | ||
|
||
class KotlinModuleTest { |
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.
Test because mis-wiring one of these booleans would be easy to do.
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.
Good idea as well.
Hmm, I'm a bit confused by those test failures. The head commit on 2.11 passed on Travis, but fails in the same way as this PR when I run it locally. |
Fixed—jackson-databind changed the default representation of TZ offsets in FasterXML/jackson-databind#2643 |
Ah yes, thank you for addressing the date/time formatting change: did not realize it but yes that would cause test fails. This is one reason why I think a release candidate (at least one) is needed for 2.11, since this is one change that could result in both real problems and failures for tests that check for exact string. |
We're going to be adding more settings in the future, create a Builder so the module can be easily constructed from a Java context.
#281 (comment)