Fred Rolland has posted comments on this change. Change subject: webadmin: Warning on dependent VMs on delete template ......................................................................
Patch Set 7: (8 comments) Fixed patch comments https://gerrit.ovirt.org/#/c/37514/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java: Line 124: @Override Line 125: public void onSuccess(Object model, Object returnValue) { Line 126: List<VM> vmsInExportDomain; Line 127: VdcQueryReturnValue retVal = (VdcQueryReturnValue) returnValue; Line 128: if (retVal != null && retVal.getReturnValue() != null && retVal.getSucceeded()) { > is the early return problematic here? Done Line 129: vmsInExportDomain = retVal.getReturnValue(); Line 130: if ((vmsInExportDomain.size() > 0)) { Line 131: TemplateBackupModel templateBackupModel = (TemplateBackupModel) model; Line 132: HashMap<String, List<String>> problematicVmNames Line 133: = getDependentVMsForTemplate(vmsInExportDomain, templateBackupModel); Line 134: if (!problematicVmNames.isEmpty()) { Line 135: showRemoveTemplateWithDependentVMConfirmationWindow(problematicVmNames); Line 136: } Line 137: else { > style - move 'else {' to previous eol Done Line 138: removeTemplateBackup(); Line 139: } Line 140: } Line 141: else { Line 137: else { Line 138: removeTemplateBackup(); Line 139: } Line 140: } Line 141: else { > same Done Line 142: removeTemplateBackup(); Line 143: } Line 144: Line 145: } Line 140: } Line 141: else { Line 142: removeTemplateBackup(); Line 143: } Line 144: > redundant empty line Done Line 145: } Line 146: stopProgress(); Line 147: } Line 148: }; Line 142: removeTemplateBackup(); Line 143: } Line 144: Line 145: } Line 146: stopProgress(); > unneeded if there's no startProgress Done Line 147: } Line 148: }; Line 149: } Line 150: Line 230: Line 231: } Line 232: }), Line 233: getEntity().getId()); Line 234: setConfirmWindow(null); > better call cancel() inside 'onSuccess' (to keep the dialog open until the Done Line 235: } Line 236: Line 237: @Override Line 238: protected ArchitectureType getArchitectureFromItem(Object item) { Line 484: } Line 485: else { Line 486: super.executeCommand(command); Line 487: } Line 488: > redundant Done Line 489: } Line 490: Line 491: @Override Line 492: protected String getListName() { https://gerrit.ovirt.org/#/c/37514/7/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java: Line 94: Line 95: @DefaultMessage("VM(s): {0} already exist on the target Export Domain. If you want to override them, please check the ''Force Override'' check-box.") Line 96: String vmsAlreadyExistOnTargetExportDomain(String existingVMs); Line 97: Line 98: @DefaultMessage("Template {0} (for VM(s) : {1} )") > formatting/phrasing: Done Line 99: String templatesWithDependentVMs(String template, String vms); Line 100: Line 101: @DefaultMessage("Shared disk(s) will not be a part of the VM export: {0}.") Line 102: String sharedDisksWillNotBePartOfTheExport(String diskList); -- To view, visit https://gerrit.ovirt.org/37514 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8cba5df5a41973741a855ae0c9efddd56100d2fd Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
