-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat: added serialization for kotlinx.datetime #373
Conversation
…calDateTime and LocalTime
Codecov ReportBase: 58.17% // Head: 58.43% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #373 +/- ##
============================================
+ Coverage 58.17% 58.43% +0.26%
- Complexity 752 753 +1
============================================
Files 135 135
Lines 5604 5640 +36
Branches 477 477
============================================
+ Hits 3260 3296 +36
- Misses 2132 2133 +1
+ Partials 212 211 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
It would be great to as a unit test like: https://github.com/Litote/kmongo/blob/master/kmongo-core-tests/src/main/kotlin/org/litote/kmongo/DateTest.kt
@@ -54,6 +54,10 @@ | |||
<groupId>org.jetbrains.kotlinx</groupId> | |||
<artifactId>kotlinx-serialization-core-jvm</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.jetbrains.kotlinx</groupId> | |||
<artifactId>kotlinx-datetime-jvm</artifactId> |
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.
add an <optional>true</optional>
KTXInstant::class to KTXInstantSerializer, | ||
KTXLocalDate::class to KTXLocalDateSerializer, | ||
KTXLocalDateTime::class to KTXLocalDateTimeSerializer, | ||
KTXLocalTime::class to KTXLocalTimeSerializer, |
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.
check if the class KTXInstant is in the classpath before adding them
serializersMap: Map<KClass<*>, KSerializer<*>> = mapOf(...) +
try {
Class.forName("kotlinx.datetime.Instant")
mapOf(
KTXInstant::class to KTXInstantSerializer,
KTXLocalDate::class to KTXLocalDateSerializer,
KTXLocalDateTime::class to KTXLocalDateTimeSerializer,
KTXLocalTime::class to KTXLocalTimeSerializer
)
}
catch(e:ClassNotFoundException) { emptyMap() }
Thanks for your response. I will implement this and update the pull request. |
…LocalDate, LocalDateTime and LocalTime
Sorry for the delay. I had much to do. At first I forgot to implement jackson serialization. I now added jackson serialization and added tests. |
LGTM. Thank you ! |
added serialization for kotlinx.datetime Instant, LocalDate, LocalDateTime and LocalTime
Fixes #323