Open agologan opened 3 years ago
Merging #152 (2b0a77f) into master (b2f2c2f) will decrease coverage by
0.04%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #152 +/- ##
============================================
- Coverage 71.95% 71.90% -0.05%
Complexity 139 139
============================================
Files 11 11
Lines 574 573 -1
Branches 75 75
============================================
- Hits 413 412 -1
Misses 118 118
Partials 43 43
Impacted Files | Coverage Δ | |
---|---|---|
.../src/main/java/com/yelp/codegen/KotlinGenerator.kt | 84.42% <ø> (ø) |
|
...ain/java/com/yelp/codegen/utils/KotlinLangUtils.kt | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b2f2c2f...2b0a77f. Read the comment docs.
This PR extends on #136. There's still work to be so external input is more then welcome:
Changes that should be discussed:
ZonedDateTime
withInstant
from kotlinx-datetime - while this logically correct it does reduce all datetime representations to Zulu timezonenumber
fromBigDecimal
todouble
- there is no official kmp support forBigDecimal
which is not great becausedouble
is an approximationobject
withJsonObject
- while usingJson*
classes is more type-safe it does modify the API heavilyAs you can see most problems are around unsupported types which we could support via the Java classes but would still require some serialization changes maybe via contextual serialization?
The last problematic point is regarding nullability. While deserialization works as expected serialization implicitly omits
null
values. As far as I can tell these could be added back if the integrator passes inencodeDefaults
: true on theJsonConfiguration
. Thex-nullable
vendor extension just statesnull
as a valid value and doesn't imply the client should explicitly sendnull
to define a property as missing so maybe this is not actually a problem.