-
Notifications
You must be signed in to change notification settings - Fork 3
Update workflows to 2.0.0 #131
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
Walkthrough
Sequence Diagram(s)sequenceDiagram
autonumber
actor Trigger as Scheduler / Manual
participant GHA as GitHub Actions
participant GV as Job: generate_version
participant NB as Reusable Workflow\[email protected]
Trigger->>GHA: schedule / workflow_dispatch
GHA->>GV: run generate_version
GV-->>GHA: output version_name
GHA->>NB: invoke reusable workflow\n(with version_name, params, secrets)
NB-->>GHA: run build & distribution steps
GHA-->>Trigger: workflow completed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
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 |
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/on_release.yml (1)
26-45: Align signing secrets and provision keystore
- Replace
SIGNING_KEYSTORE_PASSWORD,SIGNING_KEY_PASSWORD,SIGNING_KEY_ALIASwith the expectedANDROID_KEYSTORE_BASE64,ANDROID_KEYSTORE_PASSWORD,ANDROID_KEY_ALIAS,ANDROID_KEY_PASSWORDparameters.- Decode
ANDROID_KEYSTORE_BASE64at runtime into a keystore file and update Gradle’sstoreFileto point to that generated file.
🧹 Nitpick comments (2)
androidApp/build.gradle.kts (1)
27-28: Env‑based versioning LGTM. Consider coerceAtLeast(1) for extra safety on versionCode.- versionCode = System.getenv("ANDROID_BUILD_NUMBER")?.toIntOrNull() ?: 1 + versionCode = (System.getenv("ANDROID_BUILD_NUMBER")?.toIntOrNull() ?: 1).coerceAtLeast(1).github/workflows/on_cron.yml (1)
31-33: Version name uniqueness on reruns.github.run_id stays constant on rerun; include date or run_attempt/sha to avoid collisions.
- VERSION_NAME="nightly-${{ github.run_id }}" + VERSION_NAME="nightly-$(date -u +%Y%m%d)-${{ github.run_number }}-${{ github.run_attempt }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/docs_deploy.yml(0 hunks).github/workflows/on_cron.yml(1 hunks).github/workflows/on_demand_deploy.yml(0 hunks).github/workflows/on_merge_pull_request.yml(0 hunks).github/workflows/on_pull_request.yml(2 hunks).github/workflows/on_release.yml(2 hunks)androidApp/build.gradle.kts(2 hunks)buildSrc/src/main/kotlin/app/futured/kmptemplate/gradle/configuration/ProjectSettings.kt(0 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/on_merge_pull_request.yml
- .github/workflows/on_demand_deploy.yml
- .github/workflows/docs_deploy.yml
- buildSrc/src/main/kotlin/app/futured/kmptemplate/gradle/configuration/ProjectSettings.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
Max line length: 140 characters
Files:
androidApp/build.gradle.kts
🔇 Additional comments (4)
.github/workflows/on_release.yml (1)
10-10: Pinning to 2.0.0 looks good..github/workflows/on_pull_request.yml (1)
11-11: Good move pinning reusable workflows to 2.0.0.Also applies to: 16-16, 25-25
.github/workflows/on_cron.yml (2)
5-5: Cron runs Tue–Sat only. Intentional?2–6 excludes Sun/Mon. Confirm schedule.
41-43: Verify Gradle task names in on_cron.yml
EnsurepackageEnterpriseUniversalApkandappDistributionUploadEnterpriseare valid tasks in your Android project’s plugin and buildType setup to avoid nightly CI failures.
| storePassword = System.getenv("ANDROID_KEYSTORE_PASSWORD").orEmpty() | ||
| keyAlias = System.getenv("ANDROID_KEY_ALIAS").orEmpty() | ||
| keyPassword = System.getenv("ANDROID_KEY_PASSWORD").orEmpty() |
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.
Fail‑fast on missing signing env and support SIGNING_ fallback to match CI.*
Empty strings defer errors to signing time. Resolve early and align with workflow secrets.
- storePassword = System.getenv("ANDROID_KEYSTORE_PASSWORD").orEmpty()
- keyAlias = System.getenv("ANDROID_KEY_ALIAS").orEmpty()
- keyPassword = System.getenv("ANDROID_KEY_PASSWORD").orEmpty()
+ val storePwd = System.getenv("ANDROID_KEYSTORE_PASSWORD")
+ ?: System.getenv("SIGNING_KEYSTORE_PASSWORD")
+ ?: error("Missing ANDROID_KEYSTORE_PASSWORD/SIGNING_KEYSTORE_PASSWORD")
+ val alias = System.getenv("ANDROID_KEY_ALIAS")
+ ?: System.getenv("SIGNING_KEY_ALIAS")
+ ?: error("Missing ANDROID_KEY_ALIAS/SIGNING_KEY_ALIAS")
+ val keyPwd = System.getenv("ANDROID_KEY_PASSWORD")
+ ?: System.getenv("SIGNING_KEY_PASSWORD")
+ ?: error("Missing ANDROID_KEY_PASSWORD/SIGNING_KEY_PASSWORD")
+ storePassword = storePwd
+ keyAlias = alias
+ keyPassword = keyPwdOptionally drive the keystore path from env to avoid committing it:
- storeFile = file("keystore/todo_your_release_keystore.keystore")
+ val ksPath = System.getenv("ANDROID_KEYSTORE_PATH") ?: "keystore/release.keystore"
+ storeFile = file(ksPath)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| storePassword = System.getenv("ANDROID_KEYSTORE_PASSWORD").orEmpty() | |
| keyAlias = System.getenv("ANDROID_KEY_ALIAS").orEmpty() | |
| keyPassword = System.getenv("ANDROID_KEY_PASSWORD").orEmpty() | |
| val storePwd = System.getenv("ANDROID_KEYSTORE_PASSWORD") | |
| ?: System.getenv("SIGNING_KEYSTORE_PASSWORD") | |
| ?: error("Missing ANDROID_KEYSTORE_PASSWORD/SIGNING_KEYSTORE_PASSWORD") | |
| val alias = System.getenv("ANDROID_KEY_ALIAS") | |
| ?: System.getenv("SIGNING_KEY_ALIAS") | |
| ?: error("Missing ANDROID_KEY_ALIAS/SIGNING_KEY_ALIAS") | |
| val keyPwd = System.getenv("ANDROID_KEY_PASSWORD") | |
| ?: System.getenv("SIGNING_KEY_PASSWORD") | |
| ?: error("Missing ANDROID_KEY_PASSWORD/SIGNING_KEY_PASSWORD") | |
| storePassword = storePwd | |
| keyAlias = alias | |
| keyPassword = keyPwd |
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.
Falling back to error if null causes crash during Gradle Sync = during configuration phase the entire script is evaluated, so we need to provide meaningful default value even if not building release type.
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.
@matejsemancik, you're absolutely right—I overlooked the Gradle configuration phase behavior. Since the entire script is evaluated during sync regardless of the build type, using error() would indeed break the developer experience for anyone running a simple Gradle sync or building debug variants without release signing credentials.
The current .orEmpty() approach is the appropriate solution here. It allows configuration to succeed while deferring validation to the actual signing phase, where missing credentials will naturally fail if someone attempts a release build. This strikes the right balance between validation and usability.
Thanks for the clarification!
✏️ Learnings added
Learnt from: matejsemancik
PR: futuredapp/kmp-futured-template#131
File: androidApp/build.gradle.kts:51-53
Timestamp: 2025-10-07T09:54:12.559Z
Learning: In Gradle build scripts, using `error()` for missing environment variables in signing configurations causes crashes during Gradle Sync because the entire script is evaluated during the configuration phase, even when not building that specific variant. Use `.orEmpty()` or provide default values instead to allow configuration to succeed, with validation deferred to actual build/signing time.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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.
No problem, rookie mistake.
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.
@matejsemancik, appreciate the understanding! Always learning. 😊
🧠 Learnings used
Learnt from: matejsemancik
PR: futuredapp/kmp-futured-template#131
File: androidApp/build.gradle.kts:51-53
Timestamp: 2025-10-07T09:54:12.559Z
Learning: In Gradle build scripts, using `error()` for missing environment variables in signing configurations causes crashes during Gradle Sync because the entire script is evaluated during the configuration phase, even when not building that specific variant. Use `.orEmpty()` or provide default values instead to allow configuration to succeed, with validation deferred to actual build/signing time.
Generated by 🚫 Danger |
Generated by 🚫 Danger |
Summary by CodeRabbit
New Features
Chores