-
Notifications
You must be signed in to change notification settings - Fork 61
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
K2 support #1525
K2 support #1525
Conversation
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.
Added some minor comments while I try to grasp the moving parts.
packages/cinterop/src/nativeDarwin/kotlin/io/realm/kotlin/internal/interop/RealmInterop.kt
Outdated
Show resolved
Hide resolved
packages/cinterop/src/nativeDarwinTest/kotlin/io/realm/kotlin/test/CinteropTest.kt
Outdated
Show resolved
Hide resolved
packages/test-base/src/nativeDarwinTest/kotlin/io/realm/kotlin/test/darwin/CoroutineTests.kt
Outdated
Show resolved
Hide resolved
packages/test-base/src/nativeDarwinTest/kotlin/io/realm/kotlin/test/darwin/MemoryTests.kt
Show resolved
Hide resolved
packages/test-base/src/nativeDarwinTest/kotlin/io/realm/kotlin/test/darwin/MemoryTests.kt
Outdated
Show resolved
Hide resolved
packages/plugin-compiler/src/main/kotlin/io/realm/kotlin/compiler/IrUtils.kt
Outdated
Show resolved
Hide resolved
packages/plugin-compiler/src/main/kotlin/io/realm/kotlin/compiler/Identifiers.kt
Outdated
Show resolved
Hide resolved
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.
Mostly minor stuff, otherwise looking good
@@ -4,7 +4,7 @@ | |||
* None. | |||
|
|||
### Enhancements | |||
* None. | |||
* Support for experimental K2-compilation with `kotlin.experimental.tryK2=true`. (Issue [#1483](https://github.com/realm/realm-kotlin/issues/1483)) |
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.
Maybe add the restriction about companion classes and Serialization or maybe reference some other place (either issue or documentation about 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.
I created #1567 to track it. This should enable users to find and workaround the issue. Do we also need it as known issues?
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.
Probably not, my guess is that more people will find it if we just pin #1567.
CHANGELOG.md
Outdated
@@ -22,6 +22,7 @@ | |||
* Minimum Gradle version: 6.8.3. | |||
* Minimum Android Gradle Plugin version: 4.1.3. | |||
* Minimum Android SDK: 16. | |||
* Minimum R8: 8.0.27. |
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.
Any reason we are bumping the minimum?
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.
Earlier versions of R8 does not know how to read Kotlin 1.9 metadata. This will cause the consumer project to fail because it will not properly honor the configuration when obfuscating out library. https://developer.android.com/build/kotlin-support
...ti-platform/src/nativeTest/kotlin/io/realm/test/multiplatform/util/platform/PlatformUtils.kt
Show resolved
Hide resolved
packages/plugin-compiler/src/main/kotlin/io/realm/kotlin/compiler/IrUtils.kt
Outdated
Show resolved
Hide resolved
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.
Nice
Add support for Kotlin 1.9 and the experimental K2 support.
TODO
kotlinx.serializaiton
- Not doable at the moment, tracked by [K2] IllegalStateException: Multiple plugins generated nested class with same name Companion for class #1567toString
,equals
,hashCode
k2
-support - Compiler test framework supports for K2 withusek2
parameter, but will extend ci runs and clutter the project a bit so just went with some smoke integration tests for now. Should probably rather have a full test run once we have a parallel CI pipeline with Add support for Github Actions #881.gradle-plugin-test
andrealm-java-compatibility
projects with k2 support. Should probably rather have a full test run once we have a parallel CI pipeline with Add support for Github Actions #881.Closes #1483