Allon Mureinik has posted comments on this change.

Change subject: core: change storage_domain_static update policy
......................................................................


Patch Set 8:

(1 comment)

Backeng looks OK, except for a comment about the naming (see inline) - pending 
Frontend review from relevant stakeholders.

http://gerrit.ovirt.org/#/c/36151/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java:

Line 73:                 && descriptionChangeAllowed()
Line 74:                 && validateNotTheSameName();
Line 75:     }
Line 76: 
Line 77:     private boolean descriptionChangeAllowed() {
The comment (below this line) is correct - if the name is changed, we need the 
pool to be up - but the name of the method is confusing, as this does not 
pertain to chaining the SP's DESCRIPTION (in engine terms).

How about a simpler isPoolUp()?
Line 78:         // if domain is part of pool, and name changed, check that 
pool is up in
Line 79:         // order to change description in spm
Line 80:         return validate(new 
StoragePoolValidator(getStoragePool()).isUp());
Line 81:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2dfa97b1dbe047d98f9f1e7f7ec2d53ae5c8a16b
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@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