Fix build error caused by outdated jacoco gradle plugin#34
Conversation
WalkthroughThis pull request updates several Gradle configuration files. In Changes
Poem
Suggested labels
Suggested reviewers
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 (
|
|
This PR depends on: #33 |
tuancoltech
force-pushed
the
build/replace_jacoco_plugin
branch
from
a18079a to
e6523a2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
=========================================
Coverage 11.55% 11.55%
Complexity 9 9
=========================================
Files 10 10
Lines 199 199
Branches 37 37
=========================================
Hits 23 23
Misses 175 175
Partials 1 1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
gradlew (1)
71-81: Consider usingreadlinkinstead ofls -ldfor symlink resolution.
While this approach should work on many systems, scripting withls -ldcan be unpredictable on some shells or environments (e.g., BusyBox). Usingreadlink -f(if available) is often more robust for resolving symlinks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
gradle/wrapper/gradle-wrapper.properties(1 hunks)gradlew(2 hunks)gradlew.bat(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (ubuntu-latest, 17)
🔇 Additional comments (9)
gradlew (3)
1-18: Good move to make the script strictly POSIX compliant.
The addition of explicit license headers and switching the shebang to#!/bin/shpromotes better portability. This also eases maintenance by discouraging Bash- or ksh-specific features.
134-140: Great validation of the user’s environment when JAVA_HOME is not set.
The fallback tojavain PATH plus the explicit error message if it isn't found improves user experience.
218-248: Be mindful of environment variables containing newlines or special characters.
The process of splitting and reassembling viaxargsandsedcan break if variables contain multiline data or unmatched quotes. For most use cases this is acceptable, but just be aware of potential edge cases.gradlew.bat (4)
28-34: Nice cleanup of directory resolution logic.
ResolvingAPP_HOMEvia a simplefor %%i ...loop reduces ambiguity in relative paths and ensures the correct resolution of “.” and “..”.
37-37: Default JVM options
Defining initial heap settings (-Xmx64m/-Xms64m) here provides a safe baseline. If users need more memory in certain builds, they can override viaJAVA_OPTSorGRADLE_OPTS.
74-74: Streamlined execution flow
Passing all command-line arguments (%*) directly to Gradle avoids potential parsing errors and simplifies the script. This is a clean approach.
79-87: Exit code handling
Storing theERRORLEVELinEXIT_CODEprevents zero-exit overshadowing genuine errors from preceding commands. This ensures the script exits with the correct code.gradle/wrapper/gradle-wrapper.properties (2)
3-3: Upgrade to Gradle 8.8
Upgrading from7.5to8.8may introduce changes in plugin compatibility. Make sure your Jacoco and other plugins support Gradle 8.x.
4-5: Improved security and resilience with new properties
ThenetworkTimeoutandvalidateDistributionUrlproperties help mitigate slow or tampered downloads. Setting them here is a good practice when dealing with remote Gradle distributions.
|
@jo-elimu Please review this. |
| matrix: | ||
| os: [ubuntu-latest, macos-latest, windows-latest] | ||
| java: [11, 17] | ||
| java: [17] |
There was a problem hiding this comment.
| java: [17] | |
| java: [17, 21] |
There was a problem hiding this comment.
@jo-elimu I'll do this in separate changes since the main purpose of this PR is to resolve build error because of Jacoco plugin.
Anyway, some notes regarding this:
1/ Since Android Gradle build tool is currently 8.0.x, building with Java 21 will fail because it's unsupported yet with this version.
2/ Upgrading Android Gradle build tool to 8.5.x will probably cause build to fail because the new Jacoco plugin doesn't seem to support Gradle 8.5 yet.
So, for now. we should merge this PR and look for solution of this in another one.
2 participants
Uh oh!
There was an error while loading. Please reload this page.