WIP: Move release tasks to separate scripts for better reusability #154
Conversation
WalkthroughThe changes refactor the release preparation process for an Android application by replacing the existing logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant RC as releaseClean Task
participant CL as cleanRelease Method
participant RP as releasePrepare Task
participant PR as prepareRelease Method
participant RF as releasePerform Task
participant PF as performRelease Method
participant Git as Git Repository
Dev->>RC: Invoke releaseClean task
RC->>CL: Call cleanRelease()
CL->>Git: Check for relevant commit messages
CL->>Git: Remove git tags if found
CL-->>RC: Return control
Dev->>RP: Invoke releasePrepare task
RP->>PR: Call prepareRelease()
PR->>PR: Retrieve applicationId and versionName
PR->>PR: Process versionName (truncate if hyphen present)
PR->>PR: Determine version bump (bumpVersion / bumpType)
PR->>Git: Commit changes and tag release
PR-->>RP: Return control
Dev->>RF: Invoke releasePerform task
RF->>PF: Call performRelease()
PF->>Git: Push commits to remote repository
PF-->>RF: Return control
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 (
|
![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
🧹 Nitpick comments (3)
gradle/prepare_release.gradle (3)
1-7: Clarity in Version Name TruncationThe logic for extracting the
applicationIdandversionNameis clear. As an optional improvement, consider using the more idiomatic Groovy string containment method:if (versionName.contains("-")) { versionName = versionName.split("-")[0] }This improves readability.
9-18: Robustness of Build File Update and CommitThe code correctly reads the build file's text, applies a regex-based replacement to update the
versionName, and commits the change with a descriptive message. One suggestion is to review the regex pattern/versionName(\s+.*)/to ensure it matches only the intended line and won’t inadvertently replace unexpected content. If the build file format could vary, consider tightening the pattern.
53-65: Finalizing Next Development Iteration UpdateThe block correctly increments the
versionCodeand updates both theversionName(adding a-SNAPSHOTsuffix) andversionCodein the build file. To maintain consistency with the earlier commit step, consider wrapping the final commit (lines 62–63) within a try-catch block as well. For example:- grgit.add(patterns: ['app/build.gradle']) - grgit.commit(message: "[gradle-release-task] prepare for next development iteration") + grgit.add(patterns: ['app/build.gradle']) + try { + grgit.commit(message: "[gradle-release-task] prepare for next development iteration") + } catch (Exception e) { + throw new GradleException("Failed to commit next development iteration changes, error:\n $e") + }This small change will make the process more robust against commit failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/build.gradle(1 hunks)gradle/prepare_release.gradle(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
🔇 Additional comments (2)
gradle/prepare_release.gradle (1)
19-26: Git Tag Addition with Error HandlingThe block that adds a Git tag is well structured with a try-catch that rethrows any exception as a
GradleException. This ensures any issues during tagging are explicitly flagged. Good job here.app/build.gradle (1)
116-120: Simplification of the releasePrepare TaskRefactoring the
releasePreparetask to a simple delegation withprepareRelease()is a neat way to offload complex logic and improve maintainability. Please verify that theprepareReleaseclosure (set ingradle/prepare_release.gradle) is correctly loaded into the project scope at runtime. If not, you might consider explicitly referencing it via the project’s extra properties (e.g.ext.prepareRelease()).
| // Set new version name from input parameters. | ||
| def newVersionName | ||
| if (project.properties.containsKey("bumpVersion")) { | ||
| newVersionName = project.properties["bumpVersion"] | ||
| println "Bumping the version directly (bumpVersion=$newVersionName)" | ||
| } else if (project.properties.containsKey("bumpType")) { | ||
| def (major, minor, patch) = versionName.tokenize('.') | ||
| switch (bumpType) { | ||
| case "major": | ||
| major = major.toInteger() + 1 | ||
| minor = 0 | ||
| patch = 0 | ||
| break | ||
| case "minor": | ||
| minor = minor.toInteger() + 1 | ||
| break | ||
| case "patch": | ||
| patch = patch.toInteger() + 1 | ||
| break | ||
| } | ||
| newVersionName = "$major.$minor.$patch" | ||
| } else { | ||
| throw new GradleException('Either bumpType or bumpVersion parameters should be provided') | ||
| } |
There was a problem hiding this comment.
Parameter Handling for Version Bumping
There is a potential issue in the switch-case block: the variable bumpType is referenced without being explicitly defined. This could lead to runtime errors if bumpType is not already available in the scope. To improve clarity and reliability, extract the property into a local variable before using it. For instance:
- } else if (project.properties.containsKey("bumpType")) {
- def (major, minor, patch) = versionName.tokenize('.')
- switch (bumpType) {
+ } else if (project.properties.containsKey("bumpType")) {
+ def bumpType = project.properties["bumpType"]
+ def (major, minor, patch) = versionName.tokenize('.')
+ switch (bumpType) {This ensures that bumpType is defined and clarifies its provenance.
📝 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.
| // Set new version name from input parameters. | |
| def newVersionName | |
| if (project.properties.containsKey("bumpVersion")) { | |
| newVersionName = project.properties["bumpVersion"] | |
| println "Bumping the version directly (bumpVersion=$newVersionName)" | |
| } else if (project.properties.containsKey("bumpType")) { | |
| def (major, minor, patch) = versionName.tokenize('.') | |
| switch (bumpType) { | |
| case "major": | |
| major = major.toInteger() + 1 | |
| minor = 0 | |
| patch = 0 | |
| break | |
| case "minor": | |
| minor = minor.toInteger() + 1 | |
| break | |
| case "patch": | |
| patch = patch.toInteger() + 1 | |
| break | |
| } | |
| newVersionName = "$major.$minor.$patch" | |
| } else { | |
| throw new GradleException('Either bumpType or bumpVersion parameters should be provided') | |
| } | |
| // Set new version name from input parameters. | |
| def newVersionName | |
| if (project.properties.containsKey("bumpVersion")) { | |
| newVersionName = project.properties["bumpVersion"] | |
| println "Bumping the version directly (bumpVersion=$newVersionName)" | |
| } else if (project.properties.containsKey("bumpType")) { | |
| def bumpType = project.properties["bumpType"] | |
| def (major, minor, patch) = versionName.tokenize('.') | |
| switch (bumpType) { | |
| case "major": | |
| major = major.toInteger() + 1 | |
| minor = 0 | |
| patch = 0 | |
| break | |
| case "minor": | |
| minor = minor.toInteger() + 1 | |
| break | |
| case "patch": | |
| patch = patch.toInteger() + 1 | |
| break | |
| } | |
| newVersionName = "$major.$minor.$patch" | |
| } else { | |
| throw new GradleException('Either bumpType or bumpVersion parameters should be provided') | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/build.gradle (1)
120-120: Ensure Existence and Robustness ofprepareRelease()Call
The call toprepareRelease()within thereleasePreparetask now centralizes the release logic. Verify that theprepareRelease()method is properly defined in your external script and includes sufficient error handling to cover potential edge cases during version bumping and Git operations.If additional validation or context is needed regarding the implementation inside
gradle/prepare_release.gradle, please let me know.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/build.gradle(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 17)
🔇 Additional comments (1)
app/build.gradle (1)
116-117: Modularize Release Preparation via External Script
Delegating the release preparation logic to a separate Gradle script (gradle/prepare_release.gradle) improves modularity and reusability. Ensure that this external script is maintained in tandem with any future versioning requirements and that its integration is well documented.
tuancoltech
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
gradle/perform_release.gradle (1)
1-5: Type Conversion for the "force" ParameterWhen assigning the value from
project.properties["force"](line 4), consider explicitly converting it to a Boolean (e.g. using.toBoolean()) since Gradle project properties are typically strings. This will help prevent unexpected behavior when the property is set (especially if someone passes"false"accidentally).Suggested Diff:
- force = project.properties["force"] + force = project.properties["force"].toBoolean()gradle/clean_release.gradle (1)
5-23: Git Tags Cleaning Loop and Tag Extraction
- The loop correctly checks for commit messages containing "[gradle-release-task]" and performs the removal and reset operations.
- Potential Enhancement on Tag Extraction (Line 10):
The code splits the commit message on"$applicationId-"and immediately accesses the second element. This assumes that the commit message format is always valid. Consider adding a check on the length of the resulting array to prevent potentialIndexOutOfBoundsExceptionif the format unexpectedly changes.Suggested Diff:
- def tagName = headCommitMessage.split("$applicationId-")[1] - println "Removing the git tag: $tagName" - try { - grgit.tag.remove { - names = [tagName] - } - } catch (Exception e) { - println "Error while removing git tag:\n $e" - } + def splitParts = headCommitMessage.split("$applicationId-") + if(splitParts.length > 1) { + def tagName = splitParts[1] + println "Removing the git tag: $tagName" + try { + grgit.tag.remove { + names = [tagName] + } + } catch (Exception e) { + println "Error while removing git tag:\n $e" + } + } else { + println "Warning: Unable to extract tag name from commit message: $headCommitMessage" + }Overall, the loop structure and the use of
grgit.resetto updateheadCommitMessageare appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/build.gradle(2 hunks)gradle/clean_release.gradle(1 hunks)gradle/perform_release.gradle(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
🔇 Additional comments (9)
gradle/perform_release.gradle (3)
6-9: Push Operation and LoggingThe code prints a clear message before attempting the push and then performs the push using
grgit.push(force: force, tags: true). This is concise and well structured.
10-12: Exception Handling and User GuidanceThe try-catch block correctly catches any push failures and throws a
GradleExceptionwith a helpful message, suggesting the use of-Pforce=trueto override. This is clear; however, you might consider logging the full exception details (or ensuring they are captured) to aid in debugging.
13-14: Closure CompletionThe closing lines print a final "Done!" message to signal the end of the task, which provides a clear completion indicator.
gradle/clean_release.gradle (2)
1-4: Definition of the cleanRelease ClosureThe closure is defined as an extra property (
ext.cleanRelease) and obtains theapplicationIdfromandroid.defaultConfig. Ensure that when invoked, theandroidconfiguration is already available in the project context.
24-29: Final Status LoggingThe final logging (lines 25-29) cleanly informs if the repository was already clean and confirms the process completion. This enhances clarity during task execution.
app/build.gradle (4)
3-5: Modularization via External Script InclusionThe new
apply from:statements (lines 3-5) incorporate the release preparation scripts, which increases modularity and reusability. Ensure that these paths remain correct as the project structure evolves.
84-88: Simplified releaseClean TaskThe
releaseCleantask now depends onensureCleanRepoand simply calls thecleanRelease()closure. This clean delegation improves maintainability.
94-98: Simplified releasePrepare TaskThe
releasePreparetask now callsprepareRelease()within its action. This refactoring removes inline complexity, and as long as the external script is correctly implemented, it enhances readability and reuse across the project.
100-104: Simplified releasePerform TaskSimilarly, the
releasePerformtask now delegates the push logic toperformRelease(), ensuring consistency in the release process. This abstraction improves clarity and maintainability.
…ks to utils module.
tuancoltech
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/gradle-release.yml (1)
23-29: Module-Specific Release Commands & Trailing Whitespace
The updated commands now target theappandutilsmodules explicitly, which improves clarity and modularity in the release process. However, note that line 27 contains trailing whitespace as flagged by YAML lint. Removing the extra spaces will help maintain a clean YAML file.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 27-27: trailing spaces
(trailing-spaces)
app/build.gradle (2)
77-81: Task: ensureCleanRepo in App Module
TheensureCleanRepotask implemented here mirrors the functionality inutils/build.gradleby callingensureClean(). This redundancy is acceptable if the app module requires an independent cleaning check; however, consider whether consolidating to a single declaration might reduce duplication if both modules share identical criteria.
99-103: Task: releasePerform in App Module
ThereleasePerformtask follows the established pattern by ensuring a clean repository before invokingperformRelease(), which manages pushing commits. For consistency, consider aligning task registration style across modules (e.g., usingtasks.registervs. legacy task definitions).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/gradle-release.yml(1 hunks)app/build.gradle(2 hunks)gradle/ensure_clean.gradle(1 hunks)utils/build.gradle(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gradle-release.yml
[error] 27-27: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 17)
🔇 Additional comments (9)
gradle/ensure_clean.gradle (1)
1-5: Clean Repository Check Closure Implementation
The newext.ensureCleanclosure is straightforward and effectively aborts execution when the repository isn’t clean. Depending on future needs, consider adding logging or more detailed instructions (for example, suggesting a Git command to view the repository status).utils/build.gradle (5)
6-9: Applying External Release Scripts
The use ofapply from:to load external Gradle scripts (prepare_release.gradle,clean_release.gradle,perform_release.gradle, andensure_clean.gradle) enhances modularity. Please verify that these paths (using${rootProject.projectDir}) correctly point to the intended scripts in your repository structure.
62-66: Task Registration: ensureCleanRepo
TheensureCleanRepotask is registered to callensureClean(), ensuring that subsequent release tasks run only on a clean repository. This clear dependency helps prevent accidental releases from a dirty working directory.
68-73: Task Registration: releaseClean
ThereleaseCleantask is properly defined to depend onensureCleanRepoand to executecleanRelease(). This encapsulation keeps the release cleanup process modular and ensures safety checks are performed first.
79-84: Task Registration: releasePrepare
ThereleasePreparetask correctly depends onensureCleanRepoand then callsprepareRelease(). This abstraction simplifies version handling and commit operations by delegating them to an external script.
86-91: Task Registration: releasePerform
ThereleasePerformtask follows the same good practice by ensuring a clean repository before performing the release throughperformRelease(). This consistency across tasks is a plus.app/build.gradle (3)
3-6: Integrating External Release Scripts in App Module
The addition of external script calls viaapply from:in theapp/build.gradlefile (lines 3–6) aligns this module with the modularized release process. Ensure these paths resolve correctly relative to the root project directory.
83-87: Task: releaseClean in App Module
ThereleaseCleantask is defined to depend onensureCleanRepoand then invokecleanRelease(). This clear chaining of tasks reinforces safe release practices.
92-97: Task: releasePrepare in App Module
ThereleasePreparetask correctly leverages the shared pre-release check and delegates version handling and commit operations toprepareRelease().
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/gradle-build.yml (1)
31-31: Remove Trailing Whitespace
Static analysis flagged trailing spaces on this line. Please remove any extra spaces to comply with YAML linting guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
.github/workflows/gradle-release.yml (2)
27-27: Remove Trailing Whitespace
Static analysis indicates trailing whitespace on this line. Removing the extra spaces will help maintain clean YAML formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 27-27: trailing spaces
(trailing-spaces)
33-36: Review the Release Output Directory Reference
The final step prints out the release output by navigating to a hard-coded directory:cd ~/.m2/repository/ai/elimu/content_provider/utils/1.2.29 ls -lahWhile this is useful for verifying the release, note that the version
1.2.29is hard-coded. If future releases update the version, this path may become outdated. Consider parameterizing the version number or deriving it from project metadata to reduce maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/gradle-build.yml(1 hunks).github/workflows/gradle-release.yml(1 hunks)utils/build.gradle(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- utils/build.gradle
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/gradle-build.yml
[error] 31-31: trailing spaces
(trailing-spaces)
.github/workflows/gradle-release.yml
[error] 27-27: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
🔇 Additional comments (2)
.github/workflows/gradle-build.yml (1)
24-32: Addition of a New Release Job for the Utils Module
A new job named release has been added that runs onubuntu-latestand executes the command./gradlew utils:releasePrepare -PbumpType=patch echo "Release utils done!"This addition neatly isolates release preparation for the utils module from the main build process, aligning well with the objective of achieving better reusability through separate scripts.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
.github/workflows/gradle-release.yml (1)
23-29: Consolidation of Gradle Release Commands
The updated release step now sequentially executes the following commands:
./gradlew releaseClean./gradlew app:releasePrepare -PbumpType=patch./gradlew app:releasePerform./gradlew utils:releasePrepare -PbumpType=patch./gradlew utils:releasePerformThis consolidation improves clarity and modularity by handling both app and utils releases in one unified step. Please ensure that the ordering of these commands properly reflects the dependencies between the tasks.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 27-27: trailing spaces
(trailing-spaces)
tuancoltech
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/gradle-build.yml (3)
30-31: Update Java Setup Action Version
Static analysis highlights that the runner foractions/setup-java@v3is outdated. Consider upgrading to a later version (e.g.,actions/setup-java@v4) to ensure better compatibility and access to the latest features and security fixes.- uses: actions/setup-java@v3 + uses: actions/setup-java@v4🧰 Tools
🪛 actionlint (1.7.4)
30-30: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
28-28: Fix Indentation for Checkout Step
YAMLlint reports that line 28 has incorrect indentation (expected 4 spaces instead of 6). Correcting the indentation here will help avoid potential YAML parsing issues.Please adjust the indentation so that it aligns with the expected YAML structure.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 28-28: wrong indentation: expected 4 but found 6
(indentation)
42-42: Remove Trailing Spaces
There are trailing spaces detected on line 42 which can lead to YAMLlint warnings. Removing these extraneous spaces will clean up the file formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 42-42: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/gradle-build.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/gradle-build.yml
30-30: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/gradle-build.yml
[warning] 28-28: wrong indentation: expected 4 but found 6
(indentation)
[error] 42-42: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (windows-latest, 17)
🔇 Additional comments (1)
.github/workflows/gradle-build.yml (1)
25-43: Structured Release Job Implementation
The newreleasejob is well-organized and clearly separates the release-related Gradle calls into its own job. This modularization improves reusability and maintains a clean CI/CD pipeline. Please ensure that the external Gradle scripts (clean_release.gradle,prepare_release.gradle, andperform_release.gradle) referenced via the tasks (utils:releaseClean, etc.) are also updated and well tested.🧰 Tools
🪛 actionlint (1.7.4)
30-30: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[warning] 28-28: wrong indentation: expected 4 but found 6
(indentation)
[error] 42-42: trailing spaces
(trailing-spaces)
|
@tuancoltech Looks like there is an issue with a local Maven repository?: https://github.com/elimu-ai/content-provider/actions/runs/13560106067/job/37901550735?pr=154 |
@jo-elimu Yes this PR is still in-progress. I'm working on it. |
Issue Number
Purpose
Summary by CodeRabbit
Summary by CodeRabbit
utilsmodule.