Eliraz Levi has posted comments on this change.

Change subject: webadmin: allowing prefix as mask staticIP conf
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.ovirt.org/#/c/35145/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/SubnetMaskValidation.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/SubnetMaskValidation.java:

Line 17:         this.isPrefixSupported = isPrefixSupported;
Line 18:     }
Line 19: 
Line 20:     protected String getSubnetBadFormatMessage() {
Line 21:         return isPrefixSupported ? getBadMaskFormatMessage() : 
getBadNetmaskFormatMessage();
> Again the names... It's really unclear what the difference between "mask" a
Done
Line 22: 
Line 23:     }
Line 24: 
Line 25:     protected String getBadMaskFormatMessage() {


Line 47:                         .isNetmaskFormatValid(mask);
Line 48:         boolean isValidValue = isValidFormat ?
Line 49:                 (isPrefixSupported ? 
MaskValidator.getInstance().isMaskValid(mask) : MaskValidator.getInstance()
Line 50:                         .isNetmaskValid(mask)) : false;
Line 51:         if (!isValidFormat) {
> This should be moved up before the value validation - value validation is i
Done
Line 52:             return failWith(getSubnetBadFormatMessage());
Line 53:         } else if (!isValidValue) {
Line 54:             return failWith(getInvalidMask());
Line 55:         }


http://gerrit.ovirt.org/#/c/35145/8/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/SubnetMaskValidationTest.java
File 
frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/validation/SubnetMaskValidationTest.java:

Line 77:         }
Line 78:     }
Line 79: 
Line 80:     private void setupHelper(boolean isPrefixSupported) {
Line 81:         SubnetMaskValidation underTest = new 
SubnetMaskValidation(isPrefixSupported);
> isPrefixSupported should also probably be passed with the other test parame
Done
Line 82:         validation = spy(underTest);
Line 83:         doReturn(null).when(validation).getInvalidMask();
Line 84:         doReturn(null).when(validation).getBadNetmaskFormatMessage();
Line 85:         doReturn(null).when(validation).getBadMaskFormatMessage();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If909eda53a61bda5030c342156a01e53576e77cb
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@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