[#575] [#586] Migrate to build version catalog #591
Conversation
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
ryan-conway
changed the title
ryan-conway
temporarily deployed
to
template-compose
GitHub Actions
Inactive
Kover report for template-compose:🧛 Template - Compose Unit Tests Code Coverage:
|
| File | Coverage |
|---|
Modified Files Not Found In Coverage Report:
build.gradle.kts
build.gradle.kts
build.gradle.kts
build.gradle.kts
libs.versions.toml
settings.gradle.kts
Codebase cunningly covered by count Shroud 🧛
Generated by 🚫 Danger
ryan-conway
requested review from
AVI5HEK,
chornerman,
eeeeaa,
hoangnguyen92dn,
Johnsonmaung,
kaungkhantsoe,
luongvo,
manh-t,
minhnimble,
RkNaing,
sleepylee,
thawzintoe-ptut,
toby-thanathip and
win-than-htike
as code owners
WalkthroughThe pull request introduces a comprehensive refactoring of dependency and plugin management in a Gradle-based Android project. The changes transition from using custom constant objects ( Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Gradle as Gradle Build System
participant LibsCatalog as libs.versions.toml
participant BuildScript as build.gradle.kts
Dev->>LibsCatalog: Define library versions
Dev->>LibsCatalog: Define library references
Dev->>LibsCatalog: Create library bundles
Dev->>BuildScript: Use libs aliases
Gradle->>LibsCatalog: Resolve versions
Gradle->>BuildScript: Compile project
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (8)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
ryan-conway
temporarily deployed
to
template-compose
GitHub Actions
Inactive
|
Hi @ryan-conway, I was thinking about how we manage our SDK versions in the project and would like to propose a potential improvement. Here’s what I suggest we could declare in our TOML: androidMinSdk = "24"
androidTargetSdk = "35"
androidVersionCode = "5"
androidVersionName = "0.37.0"Then, we could easily refer to these in our Gradle scripts like this: compileSdk = libs.versions.androidCompileSdk.get().toInt()From my experience in previous projects, this approach has been quite effective.What do you think about this approach? Do you see any potential issues or improvements? |
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
template-compose/data/build.gradle.kts (1)
55-58: Consider adding comments to group dependenciesThe network dependencies are well-organized using bundles. Consider adding comments to separate different dependency groups for better readability:
+ // Network api(libs.bundles.retrofit) api(libs.bundles.okhttp) api(libs.moshi) implementation(libs.bundles.moshi)template-compose/app/build.gradle.kts (1)
154-160: Add back the suggested commentsConsider adding back the descriptive comments as suggested in the previous review:
+ // Unit test testImplementation(libs.bundles.unitTest) testImplementation(libs.test.turbine) + // UI test with Robolectric testImplementation(platform(libs.compose.bom)) testImplementation(libs.bundles.uiTest)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
template-compose/app/build.gradle.kts(3 hunks)template-compose/data/build.gradle.kts(2 hunks)template-compose/domain/build.gradle.kts(2 hunks)template-compose/gradle/libs.versions.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- template-compose/gradle/libs.versions.toml
- template-compose/domain/build.gradle.kts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Verify newproject script
- GitHub Check: Review pull request
- GitHub Check: Run Detekt and unit tests
🔇 Additional comments (10)
template-compose/data/build.gradle.kts (4)
2-4: LGTM! Plugin declarations correctly migrated to version catalogThe migration to version catalog syntax for plugins is well-implemented using the correct
aliassyntax.
9-9: LGTM! SDK versions correctly configured using version catalogThe SDK version configuration properly utilizes the version catalog with appropriate type conversion.
Also applies to: 12-12
49-51: LGTM! AndroidX dependencies properly structuredThe implementation correctly separates datastore from the basic AndroidX bundle, following the previous review feedback.
60-62: LGTM! Test dependencies well-organizedTest dependencies are properly structured using bundles and individual declarations as needed.
template-compose/app/build.gradle.kts (6)
4-9: Consider migrating to KSP in a follow-up PRThe plugin declarations are correctly migrated to the version catalog. As discussed in previous comments, consider migrating from kapt to KSP in a follow-up PR for better build performance.
16-18: LGTM! Version code configuration properly implementedThe version code configuration correctly uses the version catalog while maintaining the ability to override via project properties.
23-23: LGTM! SDK and version configuration properly implementedAll SDK versions and version name are correctly sourced from the version catalog.
Also applies to: 27-30
137-138: LGTM! AndroidX dependencies properly organizedThe implementation correctly separates datastore from the basic AndroidX bundle, following previous review feedback.
140-143: LGTM! Compose dependencies well-structuredThe implementation correctly uses the Compose BOM and keeps accompanist separate from the basic Compose bundle.
100-102: Verify the timeline for compose compiler migrationThe TODO comment correctly references issue #587 for the pending migration of the compose compiler version to the version catalog.
Run the following to check the issue status:
✅ Verification successful
Technical debt properly tracked for compose compiler migration
The hardcoded compose compiler version is acknowledged technical debt with a proper tracking issue (#587) that covers a comprehensive migration plan including Kotlin 2.1.0 upgrade, Compose Compiler Plugin adoption, and KSP migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash gh issue view 587Length of output: 815
ryan-conway
force-pushed
the
feature/586-migrate-to-build-version-catalog
branch
from
ed21f0c to
645f781
Compare
ryan-conway
temporarily deployed
to
template-compose
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@ryan-conway I think you can start on part 2 for |
| id(Plugins.JAVA_LIBRARY) | ||
| id(Plugins.KOTLIN_JVM) | ||
| id(Plugins.KOVER) | ||
| id(libs.plugins.javaLibrary.get().pluginId) |
There was a problem hiding this comment.
Curious we can't apply alias as below?
There was a problem hiding this comment.
alias requires that plugin provides version, but java-library has no version 🫠
To force reading it from libs.versions.toml, the workaround is to still use id and read the plugin ID directly 🙈
There was a problem hiding this comment.
@ryan-conway So we should add comment for clarification to avoid curiosity like me.hehe
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
ryan-conway
temporarily deployed
to
template-compose
GitHub Actions
Inactive
Solves #575
Solves #586
What happened 👀
libs.versions.tomlDependencies.kt,Modules.kt,Plugins.ktandVersions.ktapp,data,domainandprojectgradle files to use new version catalog fileTODO in Part 2 🏃🏻
libs.versions.tomlInsight 📝
We cannot deleteVersions.ktas we cannot migrate all versions to the version catalog cleanly 🫠COMPOSE_COMPILERversion will be removed in another PR 🙇🏻Proof Of Work 📹
App still builds and runs
Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Chores