Allon Mureinik has posted comments on this change.
Change subject: core: refactor UpdateVmCommand
......................................................................
Patch Set 1: (7 inline comments)
see inline.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
Line 64: VmStatic newVmStatic = getParameters().getVmStaticData();
rgloan - vmStatic is retreived from the database (from getVM()), not from the
parameters.
Line 70: UpdateVmNetworks();
s/DbFacade.getInstance().getVmStaticDAO()/getVmStaticDAO()/
Line 76: .booleanValue()) {
you don't need the booleanValue() - java's outboxing should handle it,
Line 167: if (StringUtils.isEmpty(vmFromParams.getvm_name())) {
rgolan - this is apache's commons.
Line 180: if (!StringHelper.EqOp(vmFromDB.getvm_name(),
vmFromParams.getvm_name())) {
Use StringUtils.equals(Str1,Str2) instead of StringHelper that should be
deprecated.
Line 203: if (!VmHandler.isMemorySizeLegal(vmFromParams.getos(),
I'd extract isMemorySizeLegal to a helper method.
Line 285: return getParameters().getVmStaticData() != null && getVm()
!= null;
what does the VmStaticData on the parameter object have to do with the vm's
existance?
--
To view, visit http://gerrit.ovirt.org/5454
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2052e1d367bb00e7cf8dc2f44fe85938d8bdd336
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
