Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12779 +/- ##
============================================
+ Coverage 17.60% 17.68% +0.08%
- Complexity 15659 15780 +121
============================================
Files 5917 5917
Lines 531394 535073 +3679
Branches 64970 66522 +1552
============================================
+ Hits 93575 94653 +1078
- Misses 427269 429785 +2516
- Partials 10550 10635 +85
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR cleans up static analysis warnings in UserVmManagerImpl and related files, including the service interface and a template interface. The changes address issues like raw types, unnecessary casts, unused variables, inefficient null checks, and missing throw keywords.
Changes:
- Removed unused imports, fields (
statsCollector,vmScheduleManager), unused constants (MAX_HTTP_GET_LENGTH, etc.), and dead code (unused constructors/methods inVmAndCountDetails,getName(),getRandomPrivateTemplateName()) - Simplified conditionals: replaced
== null ? false : valuewith directBoolean.parseBoolean, replaced!list.stream().anyMatch(...)withlist.stream().noneMatch(...), replacedsize() > 0with!isEmpty(), etc. - Fixed multiple bugs: missing
throwforInvalidParameterValueExceptionat validation, NPE in error messages using null objects' methods, removedcallerparameter from methods where it was unused, improved method signatures to remove unchecked exceptions
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java |
Major cleanup: remove unused imports/fields/methods, fix bugs (missing throw, null NPE in error messages), simplify conditionals/collections usage |
server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java |
Update test method calls to match new method signatures (removed caller parameters, replaced 1l with 1L) |
api/src/main/java/com/cloud/vm/UserVmService.java |
Remove StorageUnavailableException from method signatures, remove redundant public modifier from interface method |
api/src/main/java/com/cloud/template/VirtualMachineTemplate.java |
Add generic type parameter to getDetails() return type |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| private void closeCheckeReservation(List<CheckedReservation> checkedReservations) { |
There was a problem hiding this comment.
The method closeCheckeReservation has a typo in its name — it should be closeCheckedReservation. This is an inconsistency that will persist in the codebase and may cause confusion when searching for this method later.
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17071 |
| } catch (CloudException e) { | ||
| throw new CloudRuntimeException("Unable to contact the agent to stop the Instance " + vm, e); | ||
| } | ||
| return status; | ||
| } | ||
|
|
||
| private UserVm rebootVirtualMachine(long userId, long vmId, boolean enterSetup, boolean forced) throws InsufficientCapacityException, ResourceUnavailableException { | ||
| private UserVm rebootVirtualMachine(long vmId, boolean enterSetup, boolean forced) throws InsufficientCapacityException, ResourceUnavailableException { |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15595)
|
Description
This PR cleans static analysis warnings from UserVmManagerImpl and some in dependencies.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?