Eliraz Levi has posted comments on this change.

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


Patch Set 9:

(3 comments)

http://gerrit.ovirt.org/#/c/35145/9/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();
> This isn't "Done".
sorry done
Line 22: 
Line 23:     }
Line 24: 
Line 25:     protected String getBadMaskFormatMessage() {


Line 43:         assert value == null || value instanceof String : "This 
validation must be run on a String!";//$NON-NLS-1$
Line 44:         String mask = (String) value;
Line 45:         if (!MaskValidator.getInstance().isMaskFormatValid(mask, 
isPrefixSupported)) {
Line 46:             return failWith(getSubnetBadFormatMessage());
Line 47:         } else if (!MaskValidator.getInstance().isMaskValid(mask, 
isPrefixSupported)) {
> As I mentioned in the common patch, this should probably be constructed dif
Done
Line 48:             return failWith(getInvalidMask());
Line 49:         }
Line 50: 
Line 51:         return new ValidationResult();


http://gerrit.ovirt.org/#/c/35145/9/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 101:                 { "/31", false, false }, //$NON-NLS-1$
Line 102:                 { "/1", false, false }, //$NON-NLS-1$
Line 103: 
Line 104:                 { "255.255.0.0", true, false }, //$NON-NLS-1$
Line 105:                 { "255.255.255.255", true, false }, //$NON-NLS-1$
> You're missing a lot of cases that exist in the common test.
Done
Line 106:         });
Line 107:     }
Line 108: 


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