drop-in compatibility adjustments #210
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewparmet
commented
Nov 24, 2023
| (protokt.v1.property).wrap = "java.net.InetAddress" | ||
| ]; | ||
|
|
||
| bytes address = 1; |
There was a problem hiding this comment.
No reason to wrap this if the outer type is going to be wrapped. (Plus, it won't compile on Kotlin JS.)
andrewparmet
commented
Nov 25, 2023
| @@ -75,26 +73,12 @@ private class MessageGenerator( | |||
| .addMember(msg.fullProtobufTypeName.embed()) | |||
| .build() | |||
| ) | |||
| handleDeprecatedKtGeneratedMessage() | |||
| // put this back when handleDeprecatedKtGeneratedMessage() is removed | |||
There was a problem hiding this comment.
Just suppress deprecation on every file we generate.
andrewparmet
commented
Nov 26, 2023
|
|
||
| // do not generate DefaultImpls objects since we do not target < JVM 1.8 | ||
| // https://blog.jetbrains.com/kotlin/2020/07/kotlin-1-4-m3-generating-default-methods-in-interfaces | ||
| freeCompilerArgs = listOf("-Xjvm-default=all") |
There was a problem hiding this comment.
Article states this will eventually be the default.
ogolberg
reviewed
Dec 5, 2023
| import protokt.v1.google.protobuf.StringValue | ||
| import java.time.LocalDate | ||
|
|
||
| @AutoService(Converter::class) |
ogolberg
reviewed
Dec 5, 2023
| @@ -0,0 +1,9 @@ | |||
| protokt.v1.DurationConverter | |||
There was a problem hiding this comment.
oh, I see
maybe we can use KSP to generate this?
There was a problem hiding this comment.
Maybe. Project for another time.
ogolberg
approved these changes
Dec 5, 2023
ogolberg
pushed a commit
to ogolberg/protokt
that referenced
this pull request
Jan 2, 2024
- bump copied generated code to 0.12.1, the last version that had the old package names (includes KtGeneratedFileDescriptor annotation) - remove old protokt.proto file so that buf is happy that only one file has the protokt extension number - consolidate jvm extensions modules into multiplatform modules (puts files back in those JARs that were there before the 1.0 reorganization) - rename protokt outer class to be consistent with protobuf-java - remove kotlin-reflect from dependencies of protokt-core - remove interop between old and new generated code, as it is not needed for old generated code to function with the new runtime - undo manual adjustments to copied generated code as part of the above - kapt in multiplatform modules fails with: /Users/.../protokt/extensions/protokt-extensions-simple/build/tmp/kapt3/stubs/main/com/toasttab/protokt/ext/LocalDateStringConverter.java:3: error: incompatible types: NonExistentClass cannot be converted to Annotation @error.NonExistentClass(). Not sure what's up.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
/Users/.../protokt/extensions/protokt-extensions-simple/build/tmp/kapt3/stubs/main/com/toasttab/protokt/ext/LocalDateStringConverter.java:3: error: incompatible types: NonExistentClass cannot be converted to Annotation @error.NonExistentClass(). Not sure what's up.