Skip to content
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.

Sign up for GitHub

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

Break conventions for the demo app #352

Closed
wants to merge 2 commits into from
Closed

Break conventions for the demo app #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58806a6
break conventions for the demo app
breedx-splk May 8, 2024
3b7289e
spotless
breedx-splk May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions demo-app/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
import java.io.FileInputStream
import java.util.Properties

plugins {
id("otel.android-app-conventions")
// NOTE: We specifically do NOT use the "otel.android-app-conventions" here
// This is due to strict version requirements between the compose compiler and
// the gradle kotlin plugin.
// See https://developer.android.com/jetpack/androidx/releases/compose-kotlin
id("com.android.application")
id("org.jetbrains.kotlin.android")
id("otel.errorprone-conventions")
}

val localProperties = Properties()
localProperties.load(FileInputStream(rootProject.file("local.properties")))

val javaVersion = rootProject.extra["java_version"] as JavaVersion
val minKotlinVersion = rootProject.extra["kotlin_min_supported_version"] as KotlinVersion

android {
namespace = "io.opentelemetry.android.demo"
compileSdk = (property("android.compileSdk") as String).toInt()

defaultConfig {
applicationId = "io.opentelemetry.android.demo"
minSdk = (property("android.minSdk") as String).toInt()
targetSdk = (property("android.targetSdk") as String).toInt()
versionCode = 1
versionName = "1.0"

Expand All @@ -22,13 +35,26 @@ android {
}
}

compileOptions {
sourceCompatibility(javaVersion)
targetCompatibility(javaVersion)
isCoreLibraryDesugaringEnabled = true
}

kotlinOptions {
jvmTarget = javaVersion.toString()
apiVersion = minKotlinVersion.version
languageVersion = minKotlinVersion.version
}

buildTypes {
all {
val accessToken = localProperties["rum.access.token"] as String?
resValue("string", "rum_access_token", accessToken ?: "fakebroken")
}
release {
isMinifyEnabled = true
// TODO: Get minification working one day for compatibility testing
isMinifyEnabled = false
proguardFiles(
getDefaultProguardFile("proguard-android-optimize.txt"),
"proguard-rules.pro",
Expand All @@ -45,6 +71,9 @@ android {
}

dependencies {
// Required to be kept in sync with the compatible version of jetpack compose
// implementation("org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.23")

implementation(libs.androidx.appcompat)
implementation(libs.androidx.constraintlayout)
implementation(libs.material)
Expand All @@ -67,6 +96,8 @@ dependencies {
implementation(libs.opentelemetry.exporter.otlp)
testImplementation(libs.bundles.junit)
androidTestImplementation(libs.androidx.junit)
androidTestImplementation(libs.androidx.test.runner)
androidTestImplementation(libs.opentelemetry.sdk.testing)
debugImplementation(libs.androidx.ui.tooling)
debugImplementation(libs.androidx.ui.test.manifest)
}
Binary file modified demo-app/src/main/res/drawable/otel_icon.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 3 additions & 3 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ junit = "5.10.2"
byteBuddy = "1.14.14"
okhttp = "4.12.0"
spotless = "6.25.0"
kotlin = "1.9.24"
kotlin = "1.9.23"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with breaking the conventions within the demo app, however, I really don't like the idea of having to downgrade Kotlin (or any library for that matter), for the whole project, just to comply with a demo app's needs. It doesn't sound right, and it leaves me with questions such as, how can we automate bumping up Kotlin's version only after Compose has given its thumbs up to it? Or, what do we do if Kotlin messes up something in a release and then publishes a patch version that's critical to upgrade to but we can't because Compose hasn't tested it yet? And then after imagining all the work that it would take in the short and long run to comply with these special needs from a library that's not even part of the artifacts that this project publishes, I can't help but ask myself why is this worth it?

If the option of having independent Gradle projects that I mentioned here is not ideal either, then I'd suggest just disabling compose's check for the "right" kotlin version, which seems to be possible by using the suppressKotlinVersionCompatibilityCheck config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't like the idea of having to downgrade Kotlin (or any library for that matter), for the whole project, just to comply with a demo app's needs.

Oh, I totally agree...and whoops, I did not mean to leave that downgraded in the root! Let me see if I can get it working with the latest kotlin in the root .toml and overridden for the demo app.

I would be happy to manually bump kotlin for the demo app on-demand (eg. have renovate ignore it), if possible. Let me try.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, unsurprisingly, you can only have one version of id("org.jetbrains.kotlin.jvm") in the classpath at runtime.

Error resolving plugin [id: 'org.jetbrains.kotlin.jvm', version: '1.9.23']
> The request for this plugin could not be satisfied because the plugin is already on the classpath with an unknown version, so compatibility cannot be checked.

So yeah, I think we're back to making the demo app a separate project and including it somehow as you described.


[libraries]
opentelemetry-platform = { module = "io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom", version.ref = "opentelemetry-instrumentation" }
Expand Down Expand Up @@ -62,7 +62,7 @@ byteBuddy-plugin = { module = "net.bytebuddy:byte-buddy-gradle-plugin", version.
kotlin-plugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }

# demo-app
androidx-core-ktx = "androidx.core:core-ktx:1.13.0"
androidx-core-ktx = "androidx.core:core-ktx:1.13.1"
androidx-lifecycle-runtime-ktx = "androidx.lifecycle:lifecycle-runtime-ktx:2.7.0"
androidx-compose-bom = "androidx.compose:compose-bom:2024.05.00"
androidx-activity-compose = "androidx.activity:activity-compose:1.9.0"
Expand All @@ -86,4 +86,4 @@ junit = ["junit-jupiter-api", "junit-jupiter-engine", "junit-vintage-engine"]
[plugins]
publishPlugin = { id = "io.github.gradle-nexus.publish-plugin", version = "2.0.0" }
spotless = { id = "com.diffplug.spotless", version.ref = "spotless" }
kotlinAndroid = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" }
#kotlinAndroid = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" }