Skip to content
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

Added documentation for @KeepGeneratedSerializer feature #2669

Merged
merged 6 commits into from
Aug 1, 2024
Merged

Conversation

shanshin
Copy link
Contributor

Resolves #2660

@shanshin shanshin requested a review from sandwwraith May 13, 2024 06:30
@shanshin shanshin changed the title Add documentation for @KeepGeneratedSerializer feature Added documentation for @KeepGeneratedSerializer feature May 13, 2024
Copy link
Member

@sandwwraith sandwwraith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth adding an example with JsonTransformingSerializer(MyObj.generatedSerializer()) as well (see #1310) — likely to json.md#json-transformations section that will explain how to make a JsonTransformingSerializer a default one while delegating to generated serializer at the same time.

Also, knitCheck task fails.

@shanshin
Copy link
Contributor Author

Waiting for 2.0.20 release

docs/json.md Outdated Show resolved Hide resolved
docs/json.md Outdated Show resolved Hide resolved
docs/json.md Outdated Show resolved Hide resolved
@shanshin shanshin merged commit 1b69687 into dev Aug 1, 2024
4 checks passed
@shanshin shanshin deleted the keep-docs branch August 1, 2024 14:43
@sandwwraith
Copy link
Member

Should we add tests to runtime for this feature?

@shanshin
Copy link
Contributor Author

shanshin commented Aug 2, 2024

I think it would be safer to add #2758

*
* Used only with the [Serializable] annotation.
* Annotation is not allowed on classes involved in polymorphic serialization:
* interfaces, sealed classes, abstract classes, classes marked by [Polymorphic].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this sentence: @KeepGeneratedSerializer is not allowed with any inheritance, but your sample code uses a class with inherits from an interface. (see other comment)

Copy link
Contributor Author

@shanshin shanshin Aug 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means that you cannot specify the @KeepGeneratedSerializer annotation on the interfaces, sealed/abstract classes, but it's ok to add this annotation add to the heirs

@KeepGeneratedSerializer
@Serializable(with = BasicProjectSerializer::class)
@SerialName("basic")
data class BasicProject(override val name: String): Project()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you test your sample automatically? If not, can you add this code to your tests in #2758?
Background: I use Kotlin 2.0.20-RC with kx-serial 1.7.1, and I get this error:

java.lang.NullPointerException
	at kotlinx.serialization.SealedClassSerializer$special$$inlined$groupingBy$1.keyOf(_Collections.kt:1547)

I will also create an issue with a small reproducer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you test your sample automatically?

yes

I will also create an issue with a small reproducer.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants