KMP multi-module integration test #8
Conversation
which fails because Wire doesn't support adding resources to kmp modules
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces Kotlin Multiplatform (KMP) multi-module integration for the Wire Gradle plugin, enhancing its flexibility and compatibility with various project structures.
- Added support for KMP in
WirePlugin.kt, improving handling of different Kotlin project types and source sets - Introduced new sample projects demonstrating Android multi-module and multi-platform multi-module setups
- Modified
WireExtension.ktto include Android and Kotlin-specific configurations for better KMP compatibility - Updated
settings.gradle.ktsto include new sample projects and comment out older ones - Added Proto files for dinosaurs, geology, and location modules, showcasing inter-module dependencies in KMP context
18 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings
| buildscript { | ||
| repositories { | ||
| mavenCentral() | ||
| google() | ||
| } | ||
| dependencies { | ||
| classpath(libs.pluginz.android) | ||
| classpath("com.squareup.wire:wire-gradle-plugin") | ||
| } | ||
| } |
There was a problem hiding this comment.
style: Consider moving buildscript block to the root project's build.gradle.kts for better organization
| optional double length_meters = 3; | ||
| optional double mass_kilograms = 4; |
There was a problem hiding this comment.
style: Consider adding units to field names for clarity, e.g., 'length_meters' to 'length_in_meters'
| android { | ||
| namespace = "com.squareup.wire.android.app.multi.geology" | ||
| } |
There was a problem hiding this comment.
style: Add compileSdk, defaultConfig, and other necessary Android configurations
| kotlin { | ||
| } |
There was a problem hiding this comment.
style: Specify Android-specific options for Wire's Kotlin configuration
| classpath(libs.pluginz.android) | ||
| classpath("com.squareup.wire:wire-gradle-plugin") |
There was a problem hiding this comment.
style: Consider using a version catalog for the Wire Gradle plugin dependency
| optional double length_meters = 3; | ||
| optional double mass_kilograms = 4; |
There was a problem hiding this comment.
style: Add units to field names for clarity.
| val hostOs = System.getProperty("os.name") | ||
| val isMingwX64 = hostOs.startsWith("Windows") | ||
| val nativeTarget = when { | ||
| hostOs == "Mac OS X" -> macosX64("native") |
There was a problem hiding this comment.
style: Consider using 'macos' instead of 'Mac OS X' for broader compatibility with newer macOS versions
|
|
||
| package squareup.location; | ||
|
|
||
| option java_package = "com.squareup.location"; |
There was a problem hiding this comment.
logic: java_package option is set, but this is a JS module. Verify if this option is necessary or if it should be changed to a JS-specific option.
| // include(":wire-benchmarks") | ||
| // include(":wire-golden-files") | ||
| // include(":wire-gradle-plugin-playground") | ||
| // include(":wire-grpc-tests") | ||
| // include(":wire-protoc-compatibility-tests") |
There was a problem hiding this comment.
style: Some modules are commented out here but included earlier in the file. Check if this is intentional or if it might cause confusion.
| } | ||
| } else { | ||
| project.logger.warn("${project.displayName} doesn't have a 'main' source sets. The .proto files will not automatically be added to the artifact.") | ||
| (project.extensions.getByName("kotlin") as? KotlinProjectExtension)?.sourceSets |
There was a problem hiding this comment.
logic: This line appears to be incomplete or unnecessary. Consider removing or completing the logic.
No description provided.