Liron Aravot has posted comments on this change.

Change subject: webadmin: Warning on dependent VMs on delete template
......................................................................


Patch Set 11:

(3 comments)

https://gerrit.ovirt.org/#/c/37514/11/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 101: 
Line 102:                     @Override
Line 103:                     public void onSuccess(Object model, Object 
returnValue) {
Line 104:                         ArrayList<StoragePool> pools = 
(ArrayList<StoragePool>) returnValue;
Line 105:                         if (pools != null && pools.size() > 0) {
consider to call isEmpty() instead of checking size > 0
Line 106:                             pool = pools.get(0);
Line 107:                             getVmsFromExportDomain(pool.getId(), 
getEntity().getId());
Line 108:                         }
Line 109:                     }


Line 103:                     public void onSuccess(Object model, Object 
returnValue) {
Line 104:                         ArrayList<StoragePool> pools = 
(ArrayList<StoragePool>) returnValue;
Line 105:                         if (pools != null && pools.size() > 0) {
Line 106:                             pool = pools.get(0);
Line 107:                             getVmsFromExportDomain(pool.getId(), 
getEntity().getId());
you forgot to change it (see discussion in patch #8)
Line 108:                         }
Line 109:                     }
Line 110:                 }),
Line 111:                 getEntity().getId());


Line 170:         confirmModel.getCommands().add(cancelConfirmationUiCommand);
Line 171:     }
Line 172: 
Line 173:     private HashMap<String, List<String>> 
getDependentVMsForTemplates(List<VM> vmsInExportDomain,
Line 174:             List<VmTemplate> templates) {
we can just pass here the map, to avoid the list creation and then map creation 
from the list. consider that...
Line 175:         // Build a map between the template ID and the template 
instance
Line 176:         Map<Guid, VmTemplate> templateMap = 
Entities.businessEntitiesById(templates);
Line 177:         // Build a map between the template ID and a list of 
dependent VMs names
Line 178:         HashMap<String, List<String>> problematicVmNames = new 
HashMap<>();


-- 
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: 11
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

Reply via email to