-
Notifications
You must be signed in to change notification settings - Fork 212
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
Ci jetpack compose #657
Ci jetpack compose #657
Conversation
f9145d9
to
f93c3e1
Compare
@@ -1,3 +1,13 @@ | |||
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") | |||
|
|||
_COMPOSE_VERSION = "1.1.0-beta03" |
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 don't have a strong opinion on this, but should we maybe target the last stable instead?
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.
It's complicated because they are tied to the Kotlin compiler. I made note of that in the description. We can pass the compiler plugin flags to ignore these Kotlin/Compose version checks, but that might bring some instability.
examples/jetpack_compose/WORKSPACE
Outdated
artifacts = [ | ||
# Workaround to add missing 'sun.misc' dependencies to 'kotlinx-coroutines-core-jvm' artifact | ||
# Check root BUILD file and 'override_targets' arg of a primary 'maven_install' | ||
"org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.5.1", |
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.
Minor: bump to 1.5.2?
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.
Updated
|
||
android_sdk_repository( | ||
name = "androidsdk", | ||
api_level = 29, | ||
build_tools_version = versions.ANDROID.BUILD_TOOLS, # versions > 30.0.3 do not have the dx.jar anymore. |
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.
Might wanna link to bazelbuild/bazel#13989 for some context
@@ -0,0 +1,6 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
package="cm.ben.android.bazel.compose.example"> |
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.
Shall we change the package name to something closer to the Kotlin rules' package?
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'm ok with it.
} | ||
|
||
@Preview |
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.
Does this actually work with Android Studio and Bazel? I never got the previews to work.
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.
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.
sigh
We've got some work to do before we have a complete working system.
@@ -0,0 +1,6 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
package="cm.ben.android.bazel.compose.example"> |
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'm ok with it.
Adding Jetpack compose to CI so that we can make sure that there are no regressions there.
The Kotlin compiler version has to be pinned to the current compose version. We will likely need to update the compose sample independently as Google pushes out updates for that.