Yair Zaslavsky has posted comments on this change. Change subject: core: introduce CommandSwapper ......................................................................
Patch Set 1: (5 comments) http://gerrit.ovirt.org/#/c/32573/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmSwapper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmSwapper.java: Line 14: if (VmTemplateHandler.BLANK_VM_TEMPLATE_ID.equals(managementParams.getVmStaticData().getVmtGuid())) { Line 15: return VdcActionType.AddVmFromScratch; Line 16: } Line 17: Line 18: // if thin provision -> VdcActionType.AddVmFromTemplate why comment? where is the AddVmFromTemplate? if you keep the comment, make it clear. Line 19: // otherwise -> VdcActionType.AddVm Line 20: } Line 21: Line 22: return VdcActionType.AddVm; http://gerrit.ovirt.org/#/c/32573/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandSwapper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandSwapper.java: Line 2: Line 3: import org.ovirt.engine.core.common.action.VdcActionParametersBase; Line 4: import org.ovirt.engine.core.common.action.VdcActionType; Line 5: Line 6: public interface CommandSwapper { s/CommandSwapper/CommandResolver Line 7: Line 8: VdcActionType swap(VdcActionParametersBase parameters); Line 4: import org.ovirt.engine.core.common.action.VdcActionType; Line 5: Line 6: public interface CommandSwapper { Line 7: Line 8: VdcActionType swap(VdcActionParametersBase parameters); s/swap/resolve http://gerrit.ovirt.org/#/c/32573/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java: Line 68: } Line 69: Line 70: public static <P extends VdcActionParametersBase> CommandBase<P> createCommand(VdcActionType action, P parameters, CommandContext commandContext) { Line 71: try { Line 72: action = tryToSwap(action, parameters); s/tryToSwap/tryToReolve, but why not just - resolve? Line 73: CommandBase<P> result = instantiateCommand(action, parameters, commandContext); Line 74: Line 75: Injector.injectMembers(result); Line 76: return result; Line 84: return null; Line 85: } Line 86: } Line 87: Line 88: private static VdcActionType tryToSwap(VdcActionType action, VdcActionParametersBase parameters) { change to "resolve" - tryTo is redundant. Line 89: CommandSwapper delegator = commandSwappers.get(action); Line 90: if (delegator != null) { Line 91: action = delegator.swap(parameters); Line 92: } -- To view, visit http://gerrit.ovirt.org/32573 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia667a98d5c924876c879a8c55ab4eb1b9405fcb3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
