Migrate build toolchain to Kotlin 2.2.0 (K2 Compose plugin global)#1722
Migrate build toolchain to Kotlin 2.2.0 (K2 Compose plugin global)#1722aleksandar-apostolov wants to merge 7 commits into
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
WalkthroughUpgrades Kotlin to 2.2.0 with matching KSP (2.2.0-2.0.2), kotlinx.coroutines (1.10.2), and Hilt (2.56.2) versions. Wires the K2 Compose compiler plugin through convention plugins, removes ChangesKotlin 2.2.0 Toolchain Upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt (1)
36-40: Add explicitinternalvisibility tokotlinCompilerOptionshelper.This extension is used only within the convention plugin module and does not need public exposure. Adding
internalaligns with the guideline to prefer explicit visibility modifiers and limit API surface.Proposed fix
-fun Project.kotlinCompilerOptions(block: KotlinJvmCompilerOptions.() -> Unit) { +internal fun Project.kotlinCompilerOptions(block: KotlinJvmCompilerOptions.() -> Unit) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt` around lines 36 - 40, The kotlinCompilerOptions extension function lacks an explicit visibility modifier, making it public by default. Add the internal keyword before the fun keyword in the kotlinCompilerOptions function declaration to explicitly restrict its visibility to the convention plugin module only, aligning with the guideline to prefer explicit visibility modifiers and limit the API surface of the plugin.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt`:
- Around line 21-25: The property parsing for warningsAsErrors is incorrect
because Gradle properties are stored as Strings, not Booleans. The cast `as?
Boolean` will always return null for String values. Fix this by retrieving the
property as a String and converting it to a Boolean by checking if the string
value equals "true" (case-insensitive), then use that Boolean result to set the
`allWarningsAsErrors` property in the compilerOptions block. This ensures the
actual property value is properly parsed and passed to the Kotlin compiler.
---
Nitpick comments:
In `@build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt`:
- Around line 36-40: The kotlinCompilerOptions extension function lacks an
explicit visibility modifier, making it public by default. Add the internal
keyword before the fun keyword in the kotlinCompilerOptions function declaration
to explicitly restrict its visibility to the convention plugin module only,
aligning with the guideline to prefer explicit visibility modifiers and limit
the API surface of the plugin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b3c7b0f1-c9ca-4512-8498-1dfdc38e0e93
📒 Files selected for processing (12)
.planning/phases/01-foundation/01-01-SUMMARY.md.planning/phases/01-foundation/IGNORE-BASELINE.mdbuild-logic/convention/build.gradle.ktsbuild-logic/convention/src/main/kotlin/AndroidApplicationComposeConventionPlugin.ktbuild-logic/convention/src/main/kotlin/AndroidLibraryComposeConventionPlugin.ktbuild-logic/convention/src/main/kotlin/io/getstream/video/AndroidCompose.ktbuild-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.ktbuild.gradle.ktsgradle/libs.versions.tomlstream-video-android-core/api/stream-video-android-core.apistream-video-android-ui-compose/api/stream-video-android-ui-compose.apistream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt
| val warningsAsErrors = properties["warningsAsErrors"] as? Boolean ?: false | ||
|
|
||
| freeCompilerArgs = freeCompilerArgs + listOf( | ||
| tasks.withType<KotlinCompile>().configureEach { | ||
| compilerOptions { | ||
| allWarningsAsErrors.set(warningsAsErrors) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current parsing + property source shape.
rg -nP 'warningsAsErrors' build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt
fd -a 'gradle.properties' | xargs -I{} rg -nP '^\s*warningsAsErrors\s*=' {}Repository: GetStream/stream-video-android
Length of output: 215
🏁 Script executed:
#!/bin/bash
# Check the function signature and imports around line 21
sed -n '1,35p' build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.ktRepository: GetStream/stream-video-android
Length of output: 1121
🏁 Script executed:
#!/bin/bash
# Search for all gradle.properties files in the repository
find . -name 'gradle.properties' -type fRepository: GetStream/stream-video-android
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Check if warningsAsErrors is actually set anywhere
rg 'warningsAsErrors\s*=' --type propertiesRepository: GetStream/stream-video-android
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Check gradle.properties files for warningsAsErrors definition
cat ./gradle.properties
echo "---"
cat ./build-logic/gradle.propertiesRepository: GetStream/stream-video-android
Length of output: 2389
🏁 Script executed:
#!/bin/bash
# Search for warningsAsErrors in all gradle.properties files
rg 'warningsAsErrors' . -g '*.properties'Repository: GetStream/stream-video-android
Length of output: 56
🏁 Script executed:
#!/bin/bash
# Check how findProperty is used elsewhere in the codebase for property parsing
rg 'findProperty' build-logic/ -A 2 -B 1Repository: GetStream/stream-video-android
Length of output: 56
Fix property parsing: properties["warningsAsErrors"] as? Boolean will always be null because Gradle properties are Strings.
When properties are passed via -P flags or gradle.properties, they are stored as Strings. The type cast as? Boolean will fail for all String values, always resulting in null, making allWarningsAsErrors effectively always false.
Proposed fix
- val warningsAsErrors = properties["warningsAsErrors"] as? Boolean ?: false
+ val warningsAsErrors =
+ findProperty("warningsAsErrors")
+ ?.toString()
+ ?.toBooleanStrictOrNull()
+ ?: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val warningsAsErrors = properties["warningsAsErrors"] as? Boolean ?: false | |
| freeCompilerArgs = freeCompilerArgs + listOf( | |
| tasks.withType<KotlinCompile>().configureEach { | |
| compilerOptions { | |
| allWarningsAsErrors.set(warningsAsErrors) | |
| val warningsAsErrors = | |
| findProperty("warningsAsErrors") | |
| ?.toString() | |
| ?.toBooleanStrictOrNull() | |
| ?: false | |
| tasks.withType<KotlinCompile>().configureEach { | |
| compilerOptions { | |
| allWarningsAsErrors.set(warningsAsErrors) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build-logic/convention/src/main/kotlin/io/getstream/video/KotlinAndroid.kt`
around lines 21 - 25, The property parsing for warningsAsErrors is incorrect
because Gradle properties are stored as Strings, not Booleans. The cast `as?
Boolean` will always return null for String values. Fix this by retrieving the
property as a String and converting it to a Boolean by checking if the string
value equals "true" (case-insensitive), then use that Boolean result to set the
`allWarningsAsErrors` property in the compilerOptions block. This ensures the
actual property value is properly parsed and passed to the Kotlin compiler.
aleksandar-apostolov
force-pushed
the
gsd/phase-1-01-kotlin
branch
from
d1b155f to
7d96a28
Compare
|
2 participants
Goal
Closes AND-1238 — first step of the V2 migration's toolchain modernization. Kotlin 2.2.0 must land before stream-android-core v5 (built against 2.2.0) can be added as a dependency. KSP / coroutines / Compose-K2 bump together to avoid broken intermediate states.
Sequential stack on
develop-v2: this PR → AGP 8.7.3 + Gradle 8.10.2 → stream-android-core 5.0.0 dep +StreamClientlifecycle wiring.Implementation
Version moves in
gradle/libs.versions.toml:kotlin1.9.252.2.0ksp1.9.25-1.0.202.2.0-2.0.2kotlinxCoroutines1.9.01.10.2kotlinTestJunit1.8.202.2.0hilt2.522.56.2(KSP 2.2.0 compatibility)K2 Compose plugin (
org.jetbrains.kotlin.plugin.compose) wired globally via bothAndroidLibraryComposeConventionPluginandAndroidApplicationComposeConventionPlugin. Module-levelcomposeOptions { kotlinCompilerExtensionVersion = ... }block deleted — K2 plugin is now the single source of truth.kotlinOptions { ... }→compilerOptions { ... }DSL migration across rootbuild.gradle.ktsandKotlinAndroid.kt(K2 hard error). Subprojects block aligned to-Xjvm-default=all. One source fix inPermissionManager.kt— droppedinlineon avalconstructor param (K2 hard error).BCV
.apidumps regenerated to absorb K2 synthetic-accessor noise — net reduction of 194 lines, zero new public symbols.Testing
./gradlew clean compileDebugKotlinacross all modules + demo-app + tutorials + metrics: green../gradlew :demo-app:kspDevelopmentDebugKotlin: green after Hilt bump../gradlew testDebugUnitTest: zero failures.@Ignorecount holds at 21 baseline../gradlew :stream-video-android-ui-compose:assembleDebug -PenableComposeCompilerReports=true: produces non-emptycompose_compiler/*-classes.txt../gradlew spotlessCheckandapiCheck: green.Note:
./gradlew detektis referenced by the pre-push hook but no module declares the task. Captured as follow-up.Summary by CodeRabbit