Allon Mureinik has posted comments on this change.

Change subject: webadmin: DataCenterStorageListModel update availability 
refactor.
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/38585/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterStorageListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/DataCenterStorageListModel.java:

Line 765:             if (isMasterDomain(domain)) {
Line 766:                 isMasterPresents = true;
Line 767:             }
Line 768: 
Line 769:             if (domain.getStorageDomainType() == 
StorageDomainType.ISO) {
If it's the master, it can't be an ISO - i'd use "else if"
Line 770:                 isISOPresents = true;
Line 771:             }
Line 772: 
Line 773:             if (domain.getStorageDomainType() == 
StorageDomainType.ImportExport) {


Line 769:             if (domain.getStorageDomainType() == 
StorageDomainType.ISO) {
Line 770:                 isISOPresents = true;
Line 771:             }
Line 772: 
Line 773:             if (domain.getStorageDomainType() == 
StorageDomainType.ImportExport) {
If it's the master or an ISO it can't be an ImportExport - i'd use "else if"
Line 774:                 isBackupPresents = true;
Line 775:             }
Line 776:         }
Line 777: 


Line 771:             }
Line 772: 
Line 773:             if (domain.getStorageDomainType() == 
StorageDomainType.ImportExport) {
Line 774:                 isBackupPresents = true;
Line 775:             }
Perhaps worth adding:

  if (isMasterPresent && isISOPresent && isBackupPresent) {
      break;
  }
Line 776:         }
Line 777: 
Line 778:         getAttachISOCommand().setIsExecutionAllowed(false);
Line 779:         getAttachISOCommand().setIsExecutionAllowed(items.size() > 0 
&& isMasterPresents && !isISOPresents);


Line 794:                 VdcActionType.DeactivateStorageDomainWithOvfUpdate));
Line 795:     }
Line 796: 
Line 797:     private boolean isMasterDomain(StorageDomain domain) {
Line 798:         return domain.getStorageDomainType() == 
StorageDomainType.Master && domain.getStatus() != null
if the status is Active, by definition it cannot be null - please remove this 
redundant check.
Line 799:                 && domain.getStatus() == StorageDomainStatus.Active;
Line 800:     }
Line 801: 
Line 802:     @Override


-- 
To view, visit https://gerrit.ovirt.org/38585
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88abe054994257e2d3bd781d4d3b53aece75cd2c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@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