Kotlin
JSON: Fix mutable `classDiscriminatorMode` in config, and mark experimental in builder
I can drop either of these commits if they're wrong, but this PR makes classDiscriminatorMode a val in the configuration, and then marks it as experimental in the builder to match the config.
I can also deprecate the config setter instead if needed, though it doesn't seem like it's necessary. In public GitHub repos it's only set in the builder, and uncommonly enough that I'd think it's unnecessary
@sandwwraith tagging you since you implemented the feature. let me know if this looks right :)
While I agree that in JsonConfiguration the property should be a val, rather than a var, and this property is experimental in that location, it would still be good to do a proper deprecation of the setter.
As to marking the property experimental in the builder that is an incompatible API change to stable code. I don't think that is correct. I suspect that it was accidentally forgotten to remove the experimental annotation in the configuration (which now makes it much easier to remove the accidental declaration as var).
That sounds fair to me! I'll update the PR when I get a chance
And for the experimental annotation, I assumed it was accidentally left out from the builder since it was missing/mismatched from the start. I agree about it feeling wrong though, code written with a stable API then becoming experimental, so I don't know the right way forward
I amended the last commit to deprecate the classDiscriminatorMode setter instead of removing it. It'll be reported as a warning, but I could change it to an error if it doesn't need to be as gradual.
I think I only need clarification about the builder property being marked as experimental or not, and what the intent was there. :)
Yes, you're right — since this is a new feature, it should have been added as experimental to the builder, but it somehow slipped both mine and reviewer's attention :(. So we need to add @ExperimentalSerializationApi now and, yes, eventually remove the setter. I think we can deprecate it directly with Level.ERROR now. Can you please add this deprecation level to the PR?
Sure thing! I just amended that commit, so now it's marked as an error