Lior Vernia has posted comments on this change. Change subject: webadmin: allowing netmask as mask staticIP conf ......................................................................
Patch Set 2: (2 comments) Haven't reviewed thoroughly due to what seems to be a basic discrepancy. http://gerrit.ovirt.org/#/c/35145/2/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 47 Line 48 Line 49 Line 50 Line 51 I think your validation is different than the original validation - it seems like there are several examples of things your validation considers as invalid, and were previously considered valid, e.g.: * 128.0.0.0 * 255.128.0.0 Please understand what the old validation was doing (including looking up what constitutes a valid netmask and why), and fix your implementation of netmask utility accordingly. Line 36: return result; Line 37: Line 38: } Line 39: Line 40: // TODO elevi should i have abstract to share this function? It could be a nice touch but should be in a different patch. Do note that since Java 7 doesn't yet support metohd implementation, you'd have to turn IValidation into an abstract class and have all implementing classes extend the new class instead (it also shouldn't be called IValidation anymore, as "I" implies interface). Line 41: private ValidationResult failWith(ValidationResult result, String errorMessage) { Line 42: result.setSuccess(false); Line 43: result.setReasons(Arrays.asList(errorMessage)); Line 44: return result; -- 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: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: 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
