-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Bump kotlinx-serialization-json from 1.4.1 to 1.5.0 #31408
Bump kotlinx-serialization-json from 1.4.1 to 1.5.0 #31408
Conversation
This comment has been minimized.
This comment has been minimized.
Hey @evanchooly , looks like this Kotlin-related upgrade is breaking our tests. I'm just talking about this one as the others are not related:
|
Bumps [kotlinx-serialization-json](https://github.com/Kotlin/kotlinx.serialization) from 1.4.1 to 1.5.0. - [Release notes](https://github.com/Kotlin/kotlinx.serialization/releases) - [Changelog](https://github.com/Kotlin/kotlinx.serialization/blob/master/CHANGELOG.md) - [Commits](Kotlin/kotlinx.serialization@v1.4.1...v1.5.0) --- updated-dependencies: - dependency-name: org.jetbrains.kotlinx:kotlinx-serialization-json dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
6f61554
to
959162c
Compare
This comment has been minimized.
This comment has been minimized.
Failing Jobs - Building f780649
Full information is available in the Build summary check run. Failures⚙️ Native Tests - Misc4 #- Failing: integration-tests/opentelemetry-jdbc-instrumentation
📦 integration-tests/opentelemetry-jdbc-instrumentation✖
|
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.
Does this change work in native mode?
Asking because I don't see how the strategy classes are registered for reflection
* </ol> | ||
*/ | ||
@ConfigItem(name = "naming-strategy") | ||
public Optional<String> namingStrategy; |
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.
This doesn't really need to be optional as we are supplying a default value.
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.
Surprisingly this does work in native mode. I was surprised, too, because there were no @RegisterForReflection
annotations. I thought maybe the when
statement in the test directly referencing the custom JsonNamingStrategy
might be tripping some wires but I changed that to a String instead and it still works. I can see printlns I added in both the JVM and native tests. My hypothesis at this point is that since JsonNamingStrategy
shows up in the other parts of the API that graalvm makes a note and tracks all the subclasses it finds of that type. I honestly don't know otherwise.
As for the Optional
, does null
count as a default value? The impression I got looking at other configs was "no." It truly is an optional config element and kotlin serialization will happily chug along without it.
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.
Interesting about native mode.
As for the Optional thing, we can do it like you have it, just saying that Optional is unnecessary as you have provided a default value
Bumps kotlinx-serialization-json from 1.4.1 to 1.5.0.
Release notes
Sourced from kotlinx-serialization-json's releases.
... (truncated)
Changelog
Sourced from kotlinx-serialization-json's changelog.
... (truncated)
Commits
8a2c1c0
Prepare 1.5.0 release (#2207)88f782c
Removed redundant usages of@Serializer
annotations (#2200)e9b9064
~lower coverage percentageb6e9f4b
Merge remote-tracking branch 'origin/master' into devacb0988
Introduce HoconEncoder and HoconDecoder interfaces (#2094)90113a9
Added ability to read buffered huge strings in custom KSerializers (#2012)623dcad
Use the same message in intrinsified serializer() and reflective serializer()270b5e5
Improve message about missing polymorphic serializer2cb7f7d
Added support for null values for nullable enums in lanient mode (#2176)b454f34
Prevent class loaders from leaking when using ClassValue cache (#2175)You can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)