-
Notifications
You must be signed in to change notification settings - Fork 42
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
Making demo app independent #355
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.
LGTM. Just have question about Java usage but non-blocking
Just to add more color to the discussion in #342 , I think a separate demo app project is a better option. Consuming the extension as an external dependency like other apps would would prevent mistakes in accidentally adding a dependency or call that would not fly for folks who are not part of the project.
While this is more annoying when we're developing new features or breaking APIs given we have to change a separate project, but I think that's actually a good thing to consume the extension like everyone else does. This is the setup I'm used to anyway, and publishing and consuming artifacts through local Maven generally makes this not super painful, for me at least, as I'm used to much more painful setup haha.
demo-app/build.gradle.kts
Outdated
@@ -42,6 +45,16 @@ android { | |||
composeOptions { | |||
kotlinCompilerExtensionVersion = "1.5.13" | |||
} | |||
val javaVersion = JavaVersion.VERSION_11 |
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.
Question: do we require (desugared) Java 11 features to use the android extension? For a demo app, it'd be nice to just use Kotlin and not having to worry about Java
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 think you're right -- at least right now it looks like the demo app is all kotlin, so I would think we could remove this and see if something else in the build requires 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.
Thanks @LikeTheSalad! This is straightforward and moves us out of the current kotlin version mismatch hell and soup of build breakages.
When I took an attempt at this, I wanted to try and avoid having to help the demo-app as another IntelliJ window, and that didn't work out at all. I think this is fine tho, if not just a little awkward for otel-android devs.
* Making demo app independent * Update demo-app/build.gradle.kts * Update demo-app/build.gradle.kts --------- Co-authored-by: jason plumb <[email protected]>
Option to avoid these kinds of issues.
This option extracts the demo app into its own project and adds a dependency onto the agent project by using the composite builds config. This option seems to be more stable for the future, considering that the other option proposed below uses a configuration param that's not properly documented, and also adds a risk of Compose not being able to properly compile due to Kotlin version issues.
Some things to consider with this option are:
demo-app
in order to be able to compile the app with it.Another option is: #356