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.

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

Switch to Kotlin build scripts + version catalog #585

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

stevesoltys
Copy link
Member

No description provided.

@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch 2 times, most recently from b40f365 to 5c0e29a Compare October 13, 2023 05:04
@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch from 5c0e29a to 3a1e77b Compare October 13, 2023 05:10
@stevesoltys stevesoltys reopened this Oct 13, 2023
@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch from b5dfb92 to 75fc00d Compare October 13, 2023 05:42
# We use "strictly" to enforce the version cannot be overriden by transitive dependencies.
# We need to enforce that the versions we use are the same as AOSP to ensure compatibility.

# Kotlin versions
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Added it back.

@@ -0,0 +1,5 @@
[versions]
kotlin = "1.8.10"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should probably keep the comments for the various kotlin versions and why we are using this one, here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the AOSP link here. Any other comments you think would be good here let me know and will add it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I found the comments we had before helpful, explaining that we need to match kotlin versions of AOSP here and what Android version was using which Kotlin version, but I guess we can build this up again starting from 14.

Copy link
Collaborator

@grote grote left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks a lot Steve!


defaultConfig {
minSdk = libs.versions.minSdk.get().toInt()
targetSdk = libs.versions.targetSdk.get().toInt()
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like this is deprecated, probably because we don't need to set this for libraries.

There's some other deprecation warnings (e.g. lintOptions -> lint), maybe already address them in an extra commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed and fixed most others.

build.gradle.kts Outdated
buildscript {
repositories {
google()
jcenter()
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we still need jcenter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

build.gradle.kts Outdated
}

subprojects {
if (path == ":app" || path == ":storage:lib") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we could enable this everywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Enabled for all except demo, which has a few violations.

@stevesoltys stevesoltys force-pushed the feature/e2e-test-logs branch 3 times, most recently from 0ef70a1 to 7bd0852 Compare October 17, 2023 01:08
@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch from 75fc00d to 93e58f7 Compare October 17, 2023 02:16
@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch from e64b00d to ba50b88 Compare October 18, 2023 01:26
Base automatically changed from feature/e2e-test-logs to android14 October 18, 2023 02:12
grote
grote previously approved these changes Oct 18, 2023
@@ -0,0 +1,5 @@
[versions]
kotlin = "1.8.10"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found the comments we had before helpful, explaining that we need to match kotlin versions of AOSP here and what Android version was using which Kotlin version, but I guess we can build this up again starting from 14.

@stevesoltys stevesoltys force-pushed the feature/kotlin-gradle-build-scripts branch from ba50b88 to b131aaf Compare October 19, 2023 04:23
@stevesoltys stevesoltys merged commit 6ce2e27 into android14 Oct 19, 2023
4 checks passed
@stevesoltys stevesoltys deleted the feature/kotlin-gradle-build-scripts branch October 19, 2023 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants