Update GitCheck to include error handling and move to java 17. #18
Conversation
WalkthroughThis update improves the git check tool by adding structured error handling with new error classes, types, and handlers. The main class now returns detailed results showing success or failure, letting users handle errors more flexibly. The build system was upgraded to Java 17 with better Gradle settings and caching. Tests were added and enhanced to cover error handling and new features. Some duplicate or outdated code was cleaned up, and HTTP handling was modernized for clearer and safer network calls. Overall, it makes the tool more robust and easier to manage errors. 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 (
|
![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 (2)
gradle.properties (1)
1-4: Good performance optimizations enabled.The configuration cache, build caching, and parallel execution will improve build times. Consider adding a comment explaining why VFS watch is disabled, as this feature usually helps with incremental builds.
src/main/java/com/eternalcode/gitcheck/GitCheck.java (1)
141-180: Heads up - string matching can be fragile!Mapping errors by checking message content works but might break if error messages change. Consider having the provider throw specific exception types instead of relying on message parsing in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
build.gradle.kts(1 hunks)gradle.properties(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)src/main/java/com/eternalcode/gitcheck/GitCheck.java(3 hunks)src/main/java/com/eternalcode/gitcheck/GitCheckError.java(1 hunks)src/main/java/com/eternalcode/gitcheck/GitCheckErrorHandler.java(1 hunks)src/main/java/com/eternalcode/gitcheck/GitCheckErrorType.java(1 hunks)src/main/java/com/eternalcode/gitcheck/GitCheckException.java(1 hunks)src/main/java/com/eternalcode/gitcheck/GitCheckResult.java(3 hunks)src/main/java/com/eternalcode/gitcheck/git/GitException.java(0 hunks)src/main/java/com/eternalcode/gitcheck/git/GitRelease.java(2 hunks)src/main/java/com/eternalcode/gitcheck/git/GitReleaseProvider.java(0 hunks)src/main/java/com/eternalcode/gitcheck/git/GitRepository.java(4 hunks)src/main/java/com/eternalcode/gitcheck/git/GitTag.java(3 hunks)src/main/java/com/eternalcode/gitcheck/github/GitHubReleaseProvider.java(2 hunks)src/main/java/com/eternalcode/gitcheck/github/JSONUtil.java(1 hunks)src/main/java/com/eternalcode/gitcheck/shared/Preconditions.java(0 hunks)src/test/java/com/eternalcode/gitcheck/GitCheckErrorHandlerTest.java(1 hunks)src/test/java/com/eternalcode/gitcheck/GitCheckErrorTest.java(1 hunks)src/test/java/com/eternalcode/gitcheck/GitCheckExceptionTest.java(1 hunks)src/test/java/com/eternalcode/gitcheck/GitCheckResultTest.java(2 hunks)src/test/java/com/eternalcode/gitcheck/GitCheckTest.java(3 hunks)src/test/java/com/eternalcode/gitcheck/github/GitHubReleaseProviderTest.java(1 hunks)src/test/java/com/eternalcode/gitcheck/github/GitHubVersionProviderTest.java(0 hunks)src/test/java/com/eternalcode/gitcheck/github/JSONUtilTest.java(1 hunks)src/test/java/com/eternalcode/gitcheck/mock/MockErrorGitReleaseProvider.java(1 hunks)src/test/java/com/eternalcode/gitcheck/mock/MockGitReleaseProvider.java(0 hunks)
💤 Files with no reviewable changes (5)
- src/main/java/com/eternalcode/gitcheck/shared/Preconditions.java
- src/main/java/com/eternalcode/gitcheck/git/GitException.java
- src/main/java/com/eternalcode/gitcheck/git/GitReleaseProvider.java
- src/test/java/com/eternalcode/gitcheck/mock/MockGitReleaseProvider.java
- src/test/java/com/eternalcode/gitcheck/github/GitHubVersionProviderTest.java
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/test/java/com/eternalcode/gitcheck/github/JSONUtilTest.java (1)
src/main/java/com/eternalcode/gitcheck/github/JSONUtil.java (1)
JSONUtil(8-44)
src/main/java/com/eternalcode/gitcheck/GitCheckException.java (1)
src/main/java/com/eternalcode/gitcheck/git/GitException.java (1)
GitException(3-12)
src/test/java/com/eternalcode/gitcheck/mock/MockErrorGitReleaseProvider.java (2)
src/main/java/com/eternalcode/gitcheck/git/GitException.java (1)
GitException(3-12)src/main/java/com/eternalcode/gitcheck/git/GitRepository.java (1)
GitRepository(10-74)
src/main/java/com/eternalcode/gitcheck/git/GitTag.java (1)
src/main/java/com/eternalcode/gitcheck/shared/Preconditions.java (1)
Preconditions(3-20)
src/test/java/com/eternalcode/gitcheck/GitCheckResultTest.java (2)
src/main/java/com/eternalcode/gitcheck/GitCheckResult.java (1)
GitCheckResult(14-125)src/main/java/com/eternalcode/gitcheck/git/GitTag.java (1)
GitTag(11-71)
src/test/java/com/eternalcode/gitcheck/github/GitHubReleaseProviderTest.java (3)
src/main/java/com/eternalcode/gitcheck/git/GitException.java (1)
GitException(3-12)src/main/java/com/eternalcode/gitcheck/git/GitRelease.java (1)
GitRelease(8-109)src/main/java/com/eternalcode/gitcheck/git/GitRepository.java (1)
GitRepository(10-74)
src/main/java/com/eternalcode/gitcheck/github/JSONUtil.java (1)
src/main/java/com/eternalcode/gitcheck/git/GitTag.java (1)
GitTag(11-71)
src/main/java/com/eternalcode/gitcheck/GitCheckResult.java (1)
src/main/java/com/eternalcode/gitcheck/shared/Preconditions.java (1)
Preconditions(3-20)
🔇 Additional comments (41)
gradle/wrapper/gradle-wrapper.properties (1)
3-3: Good Gradle version upgrade!The update from 8.0.2 to 8.14.3 brings performance improvements and new features. Make sure to test the build to ensure all plugins and configurations remain compatible.
src/test/java/com/eternalcode/gitcheck/github/JSONUtilTest.java (1)
35-35: Test correctly updated to match new exception type.The change from
NoSuchElementExceptiontoIllegalArgumentExceptionproperly reflects the updatedJSONUtil.asStringbehavior.src/main/java/com/eternalcode/gitcheck/GitCheckErrorType.java (1)
1-46: Well-designed error type enum.The enum covers all common failure scenarios with clear documentation. The error types are properly categorized and will enable better error handling throughout the application.
src/main/java/com/eternalcode/gitcheck/GitCheckException.java (1)
1-17: Clean exception class implementation.The class properly extends
GitExceptionand follows the same constructor pattern. The implementation is straightforward and well-documented.src/main/java/com/eternalcode/gitcheck/git/GitRelease.java (3)
4-4: Good import organization.The import reordering helps group related imports together.
16-22: Improved constructor readability.The multi-line parameter formatting makes the constructor more readable and easier to maintain.
30-35: Better method organization.Moving the builder method closer to the constructor improves code structure and readability.
src/test/java/com/eternalcode/gitcheck/mock/MockErrorGitReleaseProvider.java (1)
8-14: Well-designed test mock.The mock properly implements the interface and provides meaningful error messages for testing. The inclusion of repository name and status code in the error message will help with debugging test failures.
src/test/java/com/eternalcode/gitcheck/GitCheckExceptionTest.java (1)
7-25: Complete exception testing.The tests cover both constructors thoroughly, verifying that message and cause are properly initialized. This ensures the exception behaves correctly when used in error handling scenarios.
src/test/java/com/eternalcode/gitcheck/GitCheckErrorTest.java (1)
7-79: Excellent test coverage.The tests comprehensively cover all GitCheckError constructors and methods. The different scenarios (with/without status code, with/without cause) ensure robust error handling. The toString() testing is particularly good for debugging support.
src/test/java/com/eternalcode/gitcheck/GitCheckResultTest.java (2)
21-25: Good use of static factory methods.Using static factory methods instead of constructors makes the tests more readable and aligns with modern Java practices.
39-83: Comprehensive failure scenario testing.The tests properly verify that accessing release or tag data on failed results throws IllegalStateException. This ensures the error handling contract is properly enforced.
src/main/java/com/eternalcode/gitcheck/GitCheckErrorHandler.java (1)
1-46: Well-designed error handling interface!The functional interface design is clean and follows good practices. The three static factory methods provide convenient implementations for common use cases.
src/test/java/com/eternalcode/gitcheck/GitCheckErrorHandlerTest.java (1)
1-66: Comprehensive test coverage!The tests cover all scenarios well, including System.err capture for console output verification. Good use of try-finally for cleanup.
src/main/java/com/eternalcode/gitcheck/git/GitTag.java (2)
26-42: Good refactoring of factory method!Moving the factory method closer to the constructor improves code organization. The enhanced JavaDoc with format examples is helpful.
55-57: Modern instanceof pattern matching!Nice use of Java 16+ pattern matching syntax. This is more concise and readable than the traditional approach.
build.gradle.kts (3)
13-14: Java 17 upgrade looks good!The Java version upgrade from 1.9 to 17 is properly configured for both source and target compatibility.
25-25: Dependency version updated!JetBrains annotations upgraded to a newer version, which is good for Java 17 compatibility.
39-41: Smart conditional URL logic!The conditional snapshot URL setting based on version suffix is a clean solution for handling different repository types.
src/main/java/com/eternalcode/gitcheck/GitCheckError.java (2)
16-34: Well-designed constructor overloading!The multiple constructors provide good flexibility while maintaining clean defaults. The parameter validation through annotations is appropriate.
55-66: Smart toString() implementation!The conditional inclusion of status code in the string representation is clean and avoids cluttering the output when not relevant.
src/main/java/com/eternalcode/gitcheck/git/GitRepository.java (2)
34-43: Nice addition of the factory method!This provides a clean way to create instances.
65-67: Good use of pattern matching!This modernizes the code nicely for Java 17.
src/main/java/com/eternalcode/gitcheck/github/GitHubReleaseProvider.java (3)
7-29: Great modernization to HttpClient!The dependency injection approach makes this much more testable.
56-63: Clear error handling!The switch statement makes each error case easy to understand.
75-77: Proper thread handling!Good job re-interrupting the thread when catching InterruptedException.
src/test/java/com/eternalcode/gitcheck/GitCheckTest.java (2)
22-31: Clear test naming!The rename to
testGetLatestReleaseSuccessmakes the intent obvious.
54-67: Good error handler testing!Nice use of AtomicReference to capture the error for verification.
src/main/java/com/eternalcode/gitcheck/GitCheckResult.java (3)
39-62: Clean factory methods!The
success()andfailure()methods make the API very clear.
82-84: Good defensive programming!Throwing exceptions when accessing data on failed results prevents misuse.
Also applies to: 96-98, 120-122
107-110: Nice use of Optional!Returning
Optional<GitCheckError>is cleaner than nullable returns.src/main/java/com/eternalcode/gitcheck/github/JSONUtil.java (5)
10-12: Nice utility class pattern!Throwing an exception in the constructor prevents instantiation, which is a good practice for utility classes.
14-17: Clean refactoring with Optional!Good use of Optional pattern to handle missing keys consistently.
19-23: Elegant functional style!Nice chaining with
mapto convert the string to GitTag.
25-29: Consistent pattern applied!Good consistency across all the conversion methods.
31-43: Well-structured helper method!Good separation of concerns - null returns empty Optional, wrong type throws exception. Clear error messages too.
src/main/java/com/eternalcode/gitcheck/GitCheck.java (5)
17-17: Good default initialization!Starting with a no-op handler is smart - prevents null checks later.
Also applies to: 34-35
37-48: Clean fluent API!Simple and effective way to set the error handler.
50-71: Clever handler chaining!Nice decorator pattern - each handler wraps the previous one. Just be aware this creates a chain that might get long with many calls.
73-95: Consistent with status code handling!Same pattern works well for error types too.
97-116: Solid error handling!Both methods handle errors consistently. The "unknown" tag in
getLatestReleasemakes sense when there's no current version to compare.Also applies to: 118-139
| public Version version() {return Version.HTTP_1_1;} | ||
|
|
||
| @Override | ||
| public Optional<SSLSession> sslSession() {return null;} |
There was a problem hiding this comment.
Fix the return type!
The method should return Optional.empty() instead of null.
- public Optional<SSLSession> sslSession() {return null;}
+ public Optional<SSLSession> sslSession() {return Optional.empty();}📝 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.
| public Optional<SSLSession> sslSession() {return null;} | |
| public Optional<SSLSession> sslSession() {return Optional.empty();} |
🤖 Prompt for AI Agents
In src/test/java/com/eternalcode/gitcheck/github/GitHubReleaseProviderTest.java
at line 154, the sslSession() method currently returns null, but it should
return Optional.empty() to correctly represent an empty Optional. Replace the
return statement to return Optional.empty() instead of null.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/build.yml (3)
20-24: Switch to the recommended Temurin distribution
setup-java@v4preferstemurin;adoptis kept only for backwards-compatibility and shows a deprecation notice.- distribution: adopt + distribution: temurin
26-33: Leverage the official Gradle cache action for cleaner YAML
gradle/gradle-build-action@v3handles paths and keys automatically, trimming eight lines and cutting maintenance overhead.- - name: Cache Gradle - uses: actions/cache@v4 - with: - path: ~/.gradle/caches - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*','**/gradle-wrapper.properties') }} - restore-keys: '${{ runner.os }}-gradle-' + - name: Cache Gradle + uses: gradle/gradle-build-action@v3
36-36: Add a trailing newline
This quiets YAML-lint’snew-line-at-end-of-filenotice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/build.yml
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
No description provided.