-
-
Notifications
You must be signed in to change notification settings - Fork 45
Gradle and android nav update #3244
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
Conversation
📝 WalkthroughWalkthroughThe changes update the project's build configuration and Gradle wrapper to use newer versions. The navigation component dependency is upgraded from version 2.8.5 to 2.9.1, and the Android Gradle plugin is updated from 8.1.1 to 8.6.1 in the build script. The Gradle wrapper properties are also modified to use Gradle version 8.7 instead of 8.1.1. No changes were made to public or exported entities, and the structure of the configuration files remains largely the same apart from version increments and minor reordering. Suggested labels
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build.gradle(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.
Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/build.gradle:294-295
Timestamp: 2025-01-27T15:18:25.903Z
Learning: The Connect base URL and API version in build.gradle are intentionally hardcoded as these values do not change with variants.
build.gradle (1)
Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/build.gradle:294-295
Timestamp: 2025-01-27T15:18:25.903Z
Learning: The Connect base URL and API version in build.gradle are intentionally hardcoded as these values do not change with variants.
🔇 Additional comments (3)
gradle/wrapper/gradle-wrapper.properties (1)
4-6: Double-check CI JDK levelGradle 8.7 requires at least JDK 17. Make sure all CI jobs and local dev containers have been bumped; otherwise builds will fail right after this PR lands.
build.gradle (2)
3-3:nav_version = 2.9.1may still be pre-releaseAt the time of writing, 2.9.x of Navigation is tagged alpha. If the project targets only stable artifacts, pinning to 2.8.5/2.8.6 would be safer.
Please confirm that:
- All downstream modules compile against the new APIs.
- Play-store policy allows shipping with alpha libraries (some orgs forbid it).
If stability is required, consider reverting to the latest stable tag.
10-10: AGP 8.6.1 → verify tool-chain compatibility8.6.x is a canary/alpha AGP line that introduces several breaking DSL/API changes and bumps the minimum Kotlin to 2.0.*. Your root script still pins Kotlin 1.8.20, which will not compile with AGP 8.6.
Actions:
-ext.kotlin_version = '1.8.20' +ext.kotlin_version = '2.0.0' // or the lowest version accepted by AGP 8.6Also ensure every module’s
plugins { id "com.android.application" VERSION }(orlibrary) has its version removed so it inherits the classpath’s 8.6.1.Failing to align Kotlin/AGP will break the build immediately.
| #Mon Jul 14 21:36:29 ICT 2025 | ||
| distributionBase=GRADLE_USER_HOME | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.1.1-bin.zip | ||
| distributionPath=wrapper/dists | ||
| zipStorePath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.7-bin.zip |
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.
💡 Verification agent
🧩 Analysis chain
Wrapper JAR must accompany the version bump
gradle-wrapper.properties now points to Gradle 8.7, but the PR doesn’t show the regenerated gradle/wrapper/gradle-wrapper.jar (and updated shell/bat scripts). Mixing an old wrapper JAR with a newer distribution URL can break builds on developer & CI machines.
Run:
and push the updated binary.
🏁 Script executed:
./gradlew wrapper --gradle-version 8.7 --distribution-type bin
git add gradle/wrapper/gradle-wrapper.jar gradle/wrapper/gradle-wrapper.propertiesLength of output: 339
Regenerate Gradle Wrapper JAR
Your gradle-wrapper.properties now points to Gradle 8.7, but the wrapper JAR wasn’t updated due to an invalid JAVA_HOME. Please:
- Set
JAVA_HOMEto a valid JDK installation (e.g.):export JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 - Run:
./gradlew wrapper --gradle-version 8.7 --distribution-type bin git add gradle/wrapper/gradle-wrapper.jar gradle/wrapper/gradle-wrapper.properties - Commit and push the updated files.
Files to verify:
- gradle/wrapper/gradle-wrapper.jar
- gradle/wrapper/gradle-wrapper.properties
This will ensure your local and CI builds use the correct wrapper version.
🤖 Prompt for AI Agents
In gradle/wrapper/gradle-wrapper.properties lines 1 to 4, the Gradle version was
updated to 8.7 but the gradle-wrapper.jar was not regenerated due to an invalid
JAVA_HOME. Fix this by setting JAVA_HOME to a valid JDK path, then run the
command to regenerate the wrapper with Gradle 8.7 and the binary distribution
type. After that, add both gradle-wrapper.jar and gradle-wrapper.properties to
git, then commit and push the changes to ensure consistency in local and CI
builds.
|
@damagatchi retest this please |
1 similar comment
|
@damagatchi retest this please |
57cc1a7 to
dd8b660
Compare
|
@damagatchi retest this please |
1 similar comment
|
@damagatchi retest this please |
|
Build is passing now, requesting a review. |
|
@shubham1g5 Build was failing before due to jenkin's issue? I mean their gradle was not updated or something similar error? |
|
@Jignesh-dimagi It was an outdated Jenkins plugin that was causing an error before for Jacoco that we have removed to fixed this. |
Product Description
Updates Gradle and Android Nav component
Corresponds to the changes here - #3243
QA Plan
Labels and Review