Allon Mureinik has posted comments on this change.

Change subject: core: not allow detach last storage connection
......................................................................


Patch Set 1:

(2 comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageConnectionValidator.java
Line 74:         List<StorageServerConnections> connectionsForDomain = 
getAllConnectionsForDomain(storageDomain.getId());
Line 75:         return isConnectionAttachedToDomain(connectionsForDomain);
Line 76:     }
Line 77: 
Line 78:     public boolean isLastConnectionForDomain(StorageDomain 
storageDomain) {
validators aren't supposed to return booleans, but ValidationResults.
Line 79:         List<StorageServerConnections> connectionsForDomain = 
getAllConnectionsForDomain(storageDomain.getId());
Line 80:         if (connectionsForDomain.size() == 1 && 
isConnectionAttachedToDomain(connectionsForDomain)) {
Line 81:             return true;
Line 82:         }


Line 82:         }
Line 83:         return false;
Line 84:     }
Line 85: 
Line 86:     private boolean isConnectionAttachedToDomain( 
List<StorageServerConnections> connectionsForDomain ) {
I'd change the name to something like "isAnyConnectionAttachedToDomain".
It'll make the samantics a bit clearer (e.g. - what happens if one connection 
is attached and the rest are not?)

Also, nit - remove the space between "(" and "List" - this is not our codestyle.
Line 87:          for (StorageServerConnections connectionForDomain : 
connectionsForDomain) {
Line 88:             if 
(connectionForDomain.getid().equals(connection.getid())) {
Line 89:                 return true;
Line 90:             }


-- 
To view, visit http://gerrit.ovirt.org/18314
To unsubscribe, visit http://gerrit.ovirt.org/settings

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