Omer Frenkel has posted comments on this change. Change subject: core: refactor UpdateVmCommand ......................................................................
Patch Set 1: (8 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java Line 64: VmStatic newVmStatic = getParameters().getVmStaticData(); yes it is checked in canDoAction (call to isVmExist()) you are right i could use @NotNull but i needed to check getVm() != null anyway so i passed. if you think i should use it anyway i will add it. Line 70: UpdateVmNetworks(); Done Line 76: .booleanValue()) { Done Line 173: boolean vmNameValidLength = isVmNameValidLength(vmFromParams); this was my thought at first, but validation framework gives a general length check, this method check length according to the vm os and values in configuration, so i think this one is more granular check that cannot be in the validation framework. Line 180: if (!StringHelper.EqOp(vmFromDB.getvm_name(), vmFromParams.getvm_name())) { Done Line 203: if (!VmHandler.isMemorySizeLegal(vmFromParams.getos(), its already a helper method Line 272: { Done Line 285: return getParameters().getVmStaticData() != null && getVm() != null; pretty much everything: getVm() != null IFF getParameters().getVmStaticData() != null if you imply the method name isn't good, please suggest something better i couldn't come up with something that is not too long to understand. -- 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 <ofren...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches