[#586] Migrate to build version catalog (2/2) #596
Conversation
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
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
WalkthroughThis pull request introduces a comprehensive refactoring of the build configuration across the sample-compose project. The changes primarily focus on modernizing the build system by transitioning to a version catalog approach using Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant BuildSystem as Gradle Build System
participant VersionCatalog as libs.versions.toml
participant Project as Project Modules
Dev->>BuildSystem: Trigger Build
BuildSystem->>VersionCatalog: Fetch Dependency Versions
VersionCatalog-->>BuildSystem: Return Versions
BuildSystem->>Project: Apply Configurations
Project-->>BuildSystem: Compile and Build
Possibly related PRs
Suggested Reviewers
Poem
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
force-pushed
the
feature/586-migrate-to-build-version-catalog-part-2
branch
from
bb327d0 to
12471bd
Compare
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: 1
🔭 Outside diff range comments (1)
sample-compose/build.gradle.kts (1)
Fix detekt configuration file path
The detekt configuration file exists at
sample-compose/detekt-config.yml, but the build script is looking for it at the wrong location. Update the config path inbuild.gradle.kts:config = files("${project.projectDir}/detekt-config.yml")🔗 Analysis chain
Line range hint
29-45: Verify detekt configuration completeness.The detekt configuration looks good, but let's verify the configuration file:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if detekt config file exists and is not empty if [ -f "detekt-config.yml" ]; then echo "detekt-config.yml exists" if [ -s "detekt-config.yml" ]; then echo "detekt-config.yml is not empty" head -n 5 "detekt-config.yml" else echo "WARNING: detekt-config.yml is empty" fi else echo "WARNING: detekt-config.yml not found" fiLength of output: 113
Script:
#!/bin/bash # Search for detekt configuration files fd -t f -e yml -e yaml detektLength of output: 244
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
sample-compose/app/build.gradle.kts(3 hunks)sample-compose/build.gradle.kts(3 hunks)sample-compose/buildSrc/src/main/java/Configurations.kt(0 hunks)sample-compose/buildSrc/src/main/java/Versions.kt(0 hunks)sample-compose/data/build.gradle.kts(3 hunks)sample-compose/domain/build.gradle.kts(1 hunks)sample-compose/gradle/libs.versions.toml(1 hunks)sample-compose/gradle/wrapper/gradle-wrapper.properties(1 hunks)sample-compose/settings.gradle.kts(1 hunks)
💤 Files with no reviewable changes (2)
- sample-compose/buildSrc/src/main/java/Versions.kt
- sample-compose/buildSrc/src/main/java/Configurations.kt
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Review pull request
- GitHub Check: Verify newproject script
- GitHub Check: Run Detekt and unit tests
🔇 Additional comments (16)
sample-compose/settings.gradle.kts (3)
1-7: LGTM! Well-structured plugin repository configuration.The repository order follows best practices (google -> mavenCentral -> gradlePluginPortal).
17-18: LGTM! Good setup for version catalog migration.The TYPESAFE_PROJECT_ACCESSORS feature enables type-safe accessors for included builds and projects, which is perfect for version catalog usage.
8-15: Verify JitPack repository usage.The JitPack repository is included, but let's verify if it's actually needed.
✅ Verification successful
JitPack repository is required
The repository is needed for resolving Chucker and Nimble Common dependencies defined in sample-compose/gradle/libs.versions.toml.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for dependencies that might be using JitPack rg -g '*.{gradle,kts,toml}' 'com\.github\.'Length of output: 2630
sample-compose/gradle/wrapper/gradle-wrapper.properties (1)
4-4: Verify Gradle 8.0 compatibility with project plugins.The upgrade from Gradle 7.5 to 8.0 is significant. Let's verify plugin compatibility:
✅ Verification successful
Gradle 8.0 upgrade is compatible with project dependencies
All project dependencies and plugins are using recent versions that are compatible with Gradle 8.0. The Android Gradle Plugin (8.1.2), Kotlin (1.9.10), and other major dependencies are from versions that fully support Gradle 8.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for plugin versions in version catalog rg -g 'libs.versions.toml' '^\s*[a-zA-Z-]+\s*=\s*"[^"]+"' # Search for any TODO/FIXME comments that might indicate Gradle compatibility issues rg -g '*.{gradle,kts}' 'TODO|FIXME|HACK|XXX'Length of output: 4626
sample-compose/build.gradle.kts (1)
3-8: LGTM! Clean plugin declarations using version catalog.The plugins are correctly declared using the version catalog with appropriate
apply falseflags.sample-compose/domain/build.gradle.kts (2)
8-9: LGTM! Appropriate Java 17 configuration.The Java version is correctly configured for both source and target compatibility.
13-16: LGTM! Clean dependency declarations using version catalog.Dependencies are well-organized using the version catalog, and the test dependencies are nicely bundled.
sample-compose/data/build.gradle.kts (2)
8-9: LGTM! Proper Android configuration with namespace.The Android configuration is correct with the namespace and SDK versions from the version catalog.
46-66: LGTM! Well-organized dependencies with clear API exposure.Dependencies are properly organized using the version catalog. The use of
apifor Retrofit, OkHttp, and Moshi bundles is appropriate as these are meant to be exposed to dependent modules.sample-compose/app/build.gradle.kts (4)
33-40: LGTM! Proper migration to version catalog for Android configurationThe changes correctly implement centralized version management using the version catalog, and the namespace declaration follows Android best practices.
122-164: LGTM! Well-structured dependency management using version catalogDependencies are properly migrated to use the version catalog and organized into logical bundles, improving maintainability.
76-81: Verify Java 17 compatibility across the projectThe upgrade to Java 17 looks good, but let's verify that all modules are consistently using Java 17 to avoid compatibility issues.
✅ Verification successful
Java 17 configuration is consistent across all active modules ✅
All active modules (
sample-composeandtemplate-compose) are properly configured to use Java 17 for source compatibility, target compatibility, and JVM target. The deprecated modules using older Java versions are in thedeprecated/directory and don't affect the active codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Java version settings across all Gradle build files echo "Checking Java version configuration across all modules..." rg -A 2 "JavaVersion\." --type kotlinLength of output: 7175
85-91: Track hardcoded Compose compiler versionThe hardcoded Compose compiler version should be addressed in issue #587. The addition of
buildConfig = trueis appropriate for the project requirements.✅ Verification successful
Compose compiler version update is properly tracked ✅
The hardcoded Compose compiler version (1.5.3) is acknowledged and being tracked in issue #587 "Migrate to latest Kotlin 2.1.0 and Compose Compiler plugin". The TODO comment serves its purpose as a temporary solution until the planned migration is completed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if issue #587 exists and is open gh issue view 587 --json state,titleLength of output: 124
sample-compose/gradle/libs.versions.toml (3)
37-92: LGTM! Well-organized library declarationsThe libraries section follows best practices with clear categorization and naming conventions.
93-131: LGTM! Effective dependency bundlingThe bundles are logically organized and provide a clean way to manage related dependencies together.
133-140: LGTM! Complete plugin declarationsAll necessary plugins are properly declared with consistent version management.
Kover report for template-compose:🧛 Template - Compose Unit Tests Code Coverage:
|
| File | Coverage |
|---|
Modified Files Not Found In Coverage Report:
Configurations.kt
build.gradle.kts
build.gradle.kts
build.gradle.kts
build.gradle.kts
bump_version.yml
bump_version.yml
create_release_pull_request.yml
gradle-wrapper.properties
libs.versions.toml
settings.gradle.kts
Codebase cunningly covered by count Shroud 🧛
Generated by 🚫 Danger
| implementation("com.google.accompanist:accompanist-permissions:${Versions.ACCOMPANIST_VERSION}") | ||
| implementation("com.google.accompanist:accompanist-systemuicontroller:${Versions.ACCOMPANIST_VERSION}") | ||
| implementation(libs.accompanist.permissions) | ||
| implementation(libs.accompanist.systemUiController) |
There was a problem hiding this comment.
I think we should create new issue to deprecate this lib. Now this officially support in androidx.activity 💁♂️ https://google.github.io/accompanist/systemuicontroller/
ryan-conway
had a problem deploying
to
template-compose
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/bump_version.yml (1)
29-29: Improve version update robustness.The version update pattern could be more robust to prevent accidental matches.
-perl -i -pe 's/androidVersionName =(.*)$/androidVersionName = "${{ github.event.inputs.newVersion }}"/g' sample-compose/gradle/libs.versions.toml +perl -i -pe 's/^androidVersionName\s*=\s*"[^"]*"$/androidVersionName = "${{ github.event.inputs.newVersion }}"/g' sample-compose/gradle/libs.versions.toml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.cicdtemplate/.github/workflows/bump_version.yml(1 hunks).cicdtemplate/.github/workflows/create_release_pull_request.yml(1 hunks).github/workflows/bump_version.yml(1 hunks)sample-compose/app/build.gradle.kts(4 hunks)sample-compose/build.gradle.kts(3 hunks)sample-compose/gradle/libs.versions.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sample-compose/gradle/libs.versions.toml
- sample-compose/app/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 (5)
sample-compose/build.gradle.kts (4)
3-11: Well-structured plugin declarations using version catalog!The migration to version catalog syntax for plugin management is clean and follows Gradle best practices. The
apply falsepattern is correctly used for plugins that should only be applied to subprojects.
32-32: LGTM: Detekt version properly migrated to version catalog!The Detekt tool version is now properly sourced from the version catalog, consistent with the migration objectives.
20-20: Verify Java 17 compatibility across modules.The JVM target has been updated to Java 17, which aligns with the PR objectives.
Let's verify that all modules are consistently configured for Java 17:
✅ Verification successful
Java 17 configuration is consistent across modules
Both
template-composeandsample-composemodules are correctly configured with Java 17 as their JVM target.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Java version configuration across all Gradle build files # Look for sourceCompatibility, targetCompatibility, and jvmTarget settings rg -A 1 "sourceCompatibility|targetCompatibility|jvmTarget" **/*.gradle.ktsLength of output: 462
Line range hint
1-48: Verify complete migration of hardcoded versions.Let's ensure no hardcoded versions remain in the build files:
✅ Verification successful
All versions successfully migrated in this file
The verification confirms that sample-compose/build.gradle.kts has successfully migrated all version declarations to the version catalog. While some hardcoded versions were found in other build files (kotlinCompilerExtensionVersion = "1.5.3"), they are outside the scope of this file review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potentially hardcoded versions in Gradle files # Look for version numbers in quotes or direct version assignments rg -g '*.gradle.kts' '(version\s*=\s*["\'][\d\.]+["\']|["\'][\d\.]+[-\.][\w\d\.-]+["\'])'Length of output: 116
Script:
#!/bin/bash # Search for version assignments and hardcoded versions in Gradle files rg -g '*.gradle.kts' 'version\s*=\s*"[0-9]' rg -g '*.gradle.kts' "version\s*=\s*'[0-9]" rg -g '*.gradle.kts' '"[0-9]+\.[0-9]+\.[0-9]+"' rg -g '*.gradle.kts' "'[0-9]+\.[0-9]+\.[0-9]+'"Length of output: 372
.cicdtemplate/.github/workflows/create_release_pull_request.yml (1)
19-19: Verify the path and improve version extraction robustness.The path to
libs.versions.tomlmight need adjustment since this workflow is in the.cicdtemplatedirectory.Consider making the version extraction more robust:
-run: echo "version=$(perl -nle 'print $1 if /androidVersionName = \"(.*)\"$/' gradle/libs.versions.toml)" >> $GITHUB_OUTPUT +run: echo "version=$(perl -nle 'print $1 if /^androidVersionName\s*=\s*\"([^\"]+)\"/' gradle/libs.versions.toml)" >> $GITHUB_OUTPUT✅ Verification successful
Template path needs to be relative to project root
The path
gradle/libs.versions.tomlis incorrect when running from.cicdtemplatebut would work correctly when this workflow template is copied to the actual project root. This is the expected behavior for a template file.The current implementation is correct for its intended use, as:
- This file is a template meant to be copied to the project root
- The path will resolve correctly in the final location
- The version extraction pattern works as verified in both existing TOML files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and location of libs.versions.toml fd --type f "libs.versions.toml$" # Check if the version pattern exists in the found files for file in $(fd --type f "libs.versions.toml$"); do echo "Checking $file:" grep -n "androidVersionName" "$file" doneLength of output: 689
Script:
#!/bin/bash # Check the workflow file content for project context echo "Workflow file content:" cat .cicdtemplate/.github/workflows/create_release_pull_request.yml # Check directory structure relative to workflow echo -e "\nDirectory structure from workflow location:" cd .cicdtemplate && find . -type f -name "libs.versions.toml"Length of output: 1080
ryan-conway
deleted the
feature/586-migrate-to-build-version-catalog-part-2
branch
#586
What happened 👀
libs.versions.toml7.5to8.0build.gradlefilesnamespacebuildConfig = truetobuildFeaturesbuild.gradlebuildScriptandallProjectsblockssettings.gradlepluginManagementanddependencyResolutionManagementblocksrootProject.nameInsight 📝
template-composeProof Of Work 📹
App still builds and runs
output.mov
Summary by CodeRabbit
Build Configuration
libs.versions.tomlDependency Management
Project Structure