Tomas Jelinek has posted comments on this change. Change subject: webadmin,userportal: clone VM ......................................................................
Patch Set 6: (8 comments) http://gerrit.ovirt.org/#/c/23806/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java: Line 473: public void executeCommand(UICommand command) Line 474: { Line 475: super.executeCommand(command); Line 476: Line 477: if (command == getCloneVmCommand()) { > just for readability/convention, better to move this condition downwards (a Done Line 478: cloneVm(); Line 479: } Line 480: if (command == getNewVmCommand()) Line 481: { Line 542: CloneVmModel model = new CloneVmModel(vm.getVM(), constants); Line 543: setWindow(model); Line 544: Line 545: model.initialize(); Line 546: model.setTitle(ConstantsManager.getInstance() > iinm, no need to break into two lines here Done Line 547: .getConstants().cloneVmTitle()); Line 548: Line 549: model.setHelpTag(HelpTag.clone_vm); Line 550: model.setHashName("clone_vm"); //$NON-NLS-1$ Line 746: VdcActionType.RunVmOnce)); Line 747: Line 748: getCloneVmCommand().setIsExecutionAllowed(selectedItem != null Line 749: && !selectedItem.isPool() Line 750: && VdcActionUtils.canExecute(new ArrayList<VM>(Arrays.asList(new VM[]{(VM) selectedItem.getEntity()})), > No need for the new array creation (new VM[]) - but it should probably be f I would leave it there for consistency reasons. But you are right, there is no need for that Line 751: VM.class, Line 752: VdcActionType.CloneVm)); Line 753: Line 754: getChangeCdCommand().setIsExecutionAllowed(selectedItem != null http://gerrit.ovirt.org/#/c/23806/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/CloneVmModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/CloneVmModel.java: Line 68: }), getCloneName().getEntity()); Line 69: Line 70: } Line 71: Line 72: private void postCloneVmNameUnique(final Model targetModel, final boolean makeCreatorExplicitOwner) { > makeCreatorExplicitOwner - no need for 'final' Done Line 73: CloneVmParameters params = new CloneVmParameters( Line 74: getVm(), Line 75: getCloneName().getEntity()); Line 76: Line 86: }, this); Line 87: } Line 88: Line 89: public boolean validate() { Line 90: int nameLength = (AsyncDataProvider.isWindowsOsType(vm.getOs()) ? AsyncDataProvider.getMaxVmNameLengthWin() > no need for the wrapping parenthesis.. Done Line 91: : AsyncDataProvider.getMaxVmNameLengthNonWin()); Line 92: Line 93: getCloneName().validateEntity( Line 94: new IValidation[]{ http://gerrit.ovirt.org/#/c/23806/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java: Line 443: consoleModelsCache = new ConsoleModelsCache(ConsoleContext.WA, this); Line 444: setConsoleHelpers(); Line 445: Line 446: setNewVmCommand(new UICommand("NewVm", this)); //$NON-NLS-1$ Line 447: setCloneVmCommand(new UICommand("NewVm", this)); //$NON-NLS-1$ > s/NewVm/CloneVm :) Line 448: setEditCommand(new UICommand("Edit", this)); //$NON-NLS-1$ Line 449: setRemoveCommand(new UICommand("Remove", this)); //$NON-NLS-1$ Line 450: setRunCommand(new UICommand("Run", this, true)); //$NON-NLS-1$ Line 451: setPauseCommand(new UICommand("Pause", this)); //$NON-NLS-1$ Line 2398: public void executeCommand(UICommand command) Line 2399: { Line 2400: super.executeCommand(command); Line 2401: Line 2402: if (command == getCloneVmCommand()) { > just for readability, probably makes sense to switch the order of these fir Done Line 2403: cloneVm(); Line 2404: } else if (command == getNewVmCommand()) Line 2405: { Line 2406: newVm(); Line 2400: super.executeCommand(command); Line 2401: Line 2402: if (command == getCloneVmCommand()) { Line 2403: cloneVm(); Line 2404: } else if (command == getNewVmCommand()) > formatter Done Line 2405: { Line 2406: newVm(); Line 2407: } Line 2408: else if (command == getEditCommand()) -- To view, visit http://gerrit.ovirt.org/23806 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ae2bca7ce33412da3881ccadc9bb1543e32ace2 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
