Allon Mureinik has posted comments on this change. Change subject: wip DON'T SUBMIT - engine: adding new support for native USB ......................................................................
Patch Set 2: (9 inline comments) Did a java review on the backend parts, see inline. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java Line 326: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java Line 217: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java Line 117: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java Line 295: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java Line 283: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java Line 60: I'd like to see this added to the command's test (without going over all of VmHandler's logic - just mock/spy it). .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java Line 437: boolean retVal = true; ENABLED_NATIVE.equals(usbPolicy) would be clearer. Line 442: } Same as above: ENABLED_LEGACY.equals(usbPolicy). .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java Line 141: There's a lot of copy-pasted tests here. Consider having three base methods (one for supported config and one for each unsupported message), and just delegate from the tests. -- To view, visit http://gerrit.ovirt.org/4121 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I25aa315fa85838e4371e232017f7d4eeee558c79 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Oved Ourfali <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
