Upgraded ShopSync from Groovy to Kotlin DSL in Gradle and changed some translations#173
Conversation
📝 WalkthroughWalkthroughReplaced Groovy Gradle scripts with Kotlin DSL across the Android project: removed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
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. Comment |
| signingConfigs { | ||
| register("release") { | ||
| keyAlias = keystoreProperties["keyAlias"] as String | ||
| keyPassword = keystoreProperties["keyPassword"] as String | ||
| storeFile = keystoreProperties["storeFile"]?.let { file(it.toString()) } | ||
| storePassword = keystoreProperties["storePassword"] as String | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
android/build.gradle.kts (1)
9-14: Consider merging duplicatesubprojectsblocks.Two separate
subprojectsblocks can be consolidated into a single block for improved readability.🔎 Proposed consolidation
rootProject.layout.buildDirectory.set(file("../build")) subprojects { project.layout.buildDirectory.set(file("${rootProject.layout.buildDirectory.get()}/${project.name}")) -} -subprojects { project.evaluationDependsOn(":app") }android/app/build.gradle.kts (1)
1-2: Unused import:FileInputStream.
FileInputStreamis imported but can be replaced with Kotlin'sinputStream()extension for cleaner code.🔎 Proposed fix
import java.util.Properties -import java.io.FileInputStreamThen update line 17:
if (keystorePropertiesFile.exists()) { - keystoreProperties.load(FileInputStream(keystorePropertiesFile)) + keystorePropertiesFile.inputStream().use { keystoreProperties.load(it) } }android/settings.gradle.kts (1)
2-8: Missing existence check forlocal.properties.Unlike the keystore loading in
build.gradle.kts, this code doesn't check iflocal.propertiesexists before reading. While Flutter typically generates this file, a fresh clone without runningflutter pub getwill fail with an unclear error.Consider adding a more descriptive error or existence check:
🔎 Proposed improvement
val flutterSdkPath: String = run { val properties = java.util.Properties() - file("local.properties").inputStream().use { properties.load(it) } + val localPropertiesFile = file("local.properties") + require(localPropertiesFile.exists()) { + "local.properties not found. Run 'flutter pub get' to generate it." + } + localPropertiesFile.inputStream().use { properties.load(it) } val flutterSdkPath = properties.getProperty("flutter.sdk") requireNotNull(flutterSdkPath) { "flutter.sdk not set in local.properties" } flutterSdkPath }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
android/app/build.gradleandroid/app/build.gradle.ktsandroid/build.gradleandroid/build.gradle.ktsandroid/settings.gradleandroid/settings.gradle.ktslib/l10n/app_zh.arb
💤 Files with no reviewable changes (4)
- lib/l10n/app_zh.arb
- android/app/build.gradle
- android/settings.gradle
- android/build.gradle
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: aadishsamir123
Repo: ASDev-Official/shopsync PR: 147
File: android/app/build.gradle:51-58
Timestamp: 2025-12-05T14:08:58.025Z
Learning: For android/app/build.gradle in the shopsync repository: Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear), YYYYYYY=versionCode. Phone uses base 300000000, wear uses base 400000000.
📚 Learning: 2025-12-15T14:57:06.393Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T14:57:06.393Z
Learning: Applies to android/app/build.gradle : Define build flavors with dimension 'platform' for phone (flavor phone) and wear (flavor wear) in Gradle configuration
Applied to files:
android/app/build.gradle.kts
📚 Learning: 2025-12-05T14:08:58.025Z
Learnt from: aadishsamir123
Repo: ASDev-Official/shopsync PR: 147
File: android/app/build.gradle:51-58
Timestamp: 2025-12-05T14:08:58.025Z
Learning: For android/app/build.gradle in the shopsync repository: Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear), YYYYYYY=versionCode. Phone uses base 300000000, wear uses base 400000000.
Applied to files:
android/app/build.gradle.kts
🔇 Additional comments (5)
android/build.gradle.kts (1)
1-18: LGTM — clean migration to Kotlin DSL.The root build configuration correctly migrates repository declarations, build directory setup, and the clean task to Kotlin DSL syntax.
android/app/build.gradle.kts (2)
54-69: Version code scheme correctly follows project convention.The version codes use the established format
XXYYYYYYYwhere phone uses base300000000and wear uses base400000000. Based on learnings, this is the expected pattern.
58-65:manifestPlaceholders.clear()removescredentialManagerEnabledset indefaultConfig.Calling
manifestPlaceholders.clear()in each product flavor (lines 58 and 65) will remove thecredentialManagerEnabled = trueplaceholder defined indefaultConfig(line 49). If Credential Manager is needed for both flavors, this may break Google Sign-In functionality.Is removing the credential manager placeholder intentional for these flavors? If not:
🔎 Proposed fix to preserve the placeholder
create("phone") { val phoneBaseVersionCode = 300000000 val phoneVersionCode = 7 dimension = "platform" - manifestPlaceholders.clear() versionName = "5.0.2-phone" versionCode = phoneBaseVersionCode + phoneVersionCode } create("wear") { val wearBaseVersionCode = 400000000 val wearVersionCode = 5 - manifestPlaceholders.clear() dimension = "platform" versionName = "1.2.3-wear" versionCode = wearBaseVersionCode + wearVersionCode }android/settings.gradle.kts (2)
19-28: Plugin declarations and module inclusion look correct.The plugins block properly declares the required plugins with
apply falsefor those applied at the app level, and the:appmodule is correctly included.
25-25: Kotlin version2.1.21is a valid official release (published May 13, 2025) and not a typo. No action required. If desired, Kotlin 2.3.0 is now available as the latest stable version (released December 16, 2025).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
android/app/build.gradle.kts (1)
34-43: Unsafe casts still present—past reviews remain valid.The signing configuration checks if
key.propertiesexists but still uses unsafe casts (as String) on property values. If the file exists but lacks required keys, this will throw aClassCastExceptionorNullPointerExceptionduring Gradle configuration.The past review suggestions to use null-safe access (
as? StringorgetProperty()) and conditionally register the signing config only when all required values are present remain applicable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/build.gradle.kts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: aadishsamir123
Repo: ASDev-Official/shopsync PR: 147
File: android/app/build.gradle:51-58
Timestamp: 2025-12-05T14:08:58.025Z
Learning: For android/app/build.gradle in the shopsync repository: Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear), YYYYYYY=versionCode. Phone uses base 300000000, wear uses base 400000000.
📚 Learning: 2025-12-15T14:57:06.412Z
Learnt from: CR
Repo: ASDev-Official/shopsync PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T14:57:06.412Z
Learning: Applies to android/app/build.gradle : Define build flavors with dimension 'platform' for phone (flavor phone) and wear (flavor wear) in Gradle configuration
Applied to files:
android/app/build.gradle.kts
📚 Learning: 2025-12-05T14:08:58.025Z
Learnt from: aadishsamir123
Repo: ASDev-Official/shopsync PR: 147
File: android/app/build.gradle:51-58
Timestamp: 2025-12-05T14:08:58.025Z
Learning: For android/app/build.gradle in the shopsync repository: Use version code format `XXYYYYYYY` where XX=platform (30=phone, 40=wear), YYYYYYY=versionCode. Phone uses base 300000000, wear uses base 400000000.
Applied to files:
android/app/build.gradle.kts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build WearOS Flavor
- GitHub Check: Tests & Coverage
- GitHub Check: Build Phone Flavor
- GitHub Check: Build Web (WASM)
- GitHub Check: Build WearOS (Debug)
- GitHub Check: Build Web (WASM)
- GitHub Check: Build Phone (Debug)
This pull request migrates the Android build configuration from Groovy-based Gradle files to Kotlin DSL (
.kts) equivalents. The migration modernizes the build setup, improves type safety, and aligns with current best practices for Android and Flutter projects. Additionally, the version codes and names for both phone and wear product flavors have been incremented.Build System Migration:
android/app/build.gradlewithandroid/app/build.gradle.kts, converting all configuration to Kotlin DSL and updating phone and wear version codes/names. [1] [2]android/build.gradlewithandroid/build.gradle.kts, converting project structure and clean task definitions to Kotlin DSL. [1] [2]android/settings.gradlewithandroid/settings.gradle.kts, migrating plugin management and plugin declarations to Kotlin DSL. [1] [2]Localization:
lib/l10n/app_zh.arbfile.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.