Skip to content

Conversation

@DropSnorz
Copy link
Owner

@DropSnorz DropSnorz commented Dec 1, 2025

Relates to #402

@DropSnorz DropSnorz self-assigned this Dec 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request refactors the update checking mechanism by renaming UpdateService to AppUpdateService, relocating it to the core services package, replacing the RemoteVersion POJO with direct GitHub API response parsing, and updating MainController to use asynchronous CompletableFuture instead of synchronous Task-based operations for version status checks.

Changes

Cohort / File(s) Summary
Update service refactoring
owlplug-client/src/main/java/com/owlplug/core/services/AppUpdateService.java
Renamed class from UpdateService to AppUpdateService; moved package from com.owlplug.plugin.services to com.owlplug.core.services; replaced RemoteVersion-based fetch with direct GitHub releases API integration; getLastVersion() now extracts tag_name from GitHub API response map; isUpToDate() uses local variable and Semver comparison instead of RemoteVersion flow; updated error handling for RestClientException.
MainController async update
owlplug-client/src/main/java/com/owlplug/core/controllers/MainController.java
Renamed field from updateService to appUpdateService and changed type to AppUpdateService; replaced synchronous Task-based update status check with asynchronous CompletableFuture using virtual-thread executor; UI updates now via Platform.runLater() when status is available.
Removed RemoteVersion POJO
owlplug-client/src/main/java/com/owlplug/core/model/json/RemoteVersion.java
File deleted; contained simple POJO with version field and no-arg constructor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • AppUpdateService: Review GitHub API integration logic and Semver version comparison to ensure correctness
  • MainController: Verify CompletableFuture async pattern and Platform.runLater() UI updates are properly synchronized and handle edge cases
  • Class deletion: Verify no remaining references to RemoteVersion exist in codebase

Possibly related issues

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: migrating from RemoteVersion-based update status checks to GitHub releases API for retrieving the latest app version.
Description check ✅ Passed The description references issue #402, which contextualizes the change. While minimal, it is related to the changeset.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DropSnorz
Copy link
Owner Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
owlplug-client/src/main/java/com/owlplug/core/controllers/MainController.java (1)

159-169: Tighten executor lifecycle and async error handling for update check

The virtual-thread executor is created in initialize() and never closed or reused, and any exception thrown from appUpdateService.isUpToDate() would be silently dropped by the CompletableFuture. Consider either:

  • Reusing a shared, bean-managed executor (or existing TaskRunner) instead of creating a new one here, and/or
  • Using try‑with‑resources or another explicit shutdown strategy for the executor, and
  • Adding an .exceptionally(...) (or similar) stage to log failures and decide whether to treat errors as “no update” or show a warning.

This keeps resource usage predictable and makes update-check failures diagnosable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1a7939 and 68b19e8.

📒 Files selected for processing (3)
  • owlplug-client/src/main/java/com/owlplug/core/controllers/MainController.java (3 hunks)
  • owlplug-client/src/main/java/com/owlplug/core/model/json/RemoteVersion.java (0 hunks)
  • owlplug-client/src/main/java/com/owlplug/core/services/AppUpdateService.java (2 hunks)
💤 Files with no reviewable changes (1)
  • owlplug-client/src/main/java/com/owlplug/core/model/json/RemoteVersion.java
⏰ 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). (1)
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
owlplug-client/src/main/java/com/owlplug/core/controllers/MainController.java (1)

40-46: AppUpdateService wiring and imports look consistent

The switch to AppUpdateService and the added async/concurrency imports are coherent with the new update-check flow; naming and autowiring are clear and consistent.

Also applies to: 84-84

@DropSnorz DropSnorz force-pushed the chore/version-fetch branch 3 times, most recently from fd4e3f3 to adcc6f6 Compare December 1, 2025 21:55
@DropSnorz DropSnorz force-pushed the chore/version-fetch branch from adcc6f6 to ea27b40 Compare December 1, 2025 21:57
@DropSnorz DropSnorz merged commit db49454 into master Dec 1, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants