Eliraz Levi has posted comments on this change.

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


Patch Set 4:

(4 comments)

http://gerrit.ovirt.org/#/c/35145/4/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 21:         return isPrefixSupported ? ConstantsManager.getInstance()
Line 22:                 .getConstants()
Line 23:                 
.thisFieldMustContainSubnetFormatAsNetmaskOrPrefixMsg() : 
ConstantsManager.getInstance()
Line 24:                 .getConstants()
Line 25:                 
.thisFieldMustContainSubnetFormatAsNetmaskOrPrefixFreeMsg();
> Again with the "PrefixFree", why not just keep it as "thisFieldMustContainS
Done
Line 26: 
Line 27:     }
Line 28: 
Line 29:     protected String getInvalidNetmaskMessage() {


Line 36:         String mask = (String) value;
Line 37:         boolean isValidFormat =
Line 38:                 isPrefixSupported ? 
MaskValidator.getInstance().isMaskFormatValid(mask) : 
MaskValidator.getInstance()
Line 39:                         .isMaskValidFormatPrefixFree(mask);
Line 40:         boolean isValidValue =
> You should only call this in case the format is valid. That's why you can d
Done
Line 41:                 isPrefixSupported ? 
MaskValidator.getInstance().isMaskValid(mask) : MaskValidator.getInstance()
Line 42:                         .isMaskValidPrefixFree(mask);
Line 43:         if (!isValidFormat) {
Line 44:             return failWith(getSubnetBadFormatMessage());


http://gerrit.ovirt.org/#/c/35145/4/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java
File 
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java:

Line 897
Line 898
Line 899
Line 900
Line 901
> So basically you still need this exact error message, and there's no need t
i rename it and wrote a bit more detailed message


Line 896: 
Line 897:     @DefaultStringValue("This field can be empty or contain an IP 
address in format xxx.xxx.xxx.xxx")
Line 898:     String emptyOrValidIPaddressInFormatMsg();
Line 899: 
Line 900:     @DefaultStringValue("This field must contain a subnet in either 
of the following formats:\n\txxx.xxx.xxx.xxx where xxx is between 0 and 
255\n\txx where xx is between 0-32 (/ prefix is optional, etc. /15 ).")
> You don't have to mention the optional '/' character in an error message. Y
Done
Line 901:     String thisFieldMustContainSubnetFormatAsNetmaskOrPrefixMsg();
Line 902: 
Line 903:     @DefaultStringValue("This field must contain a subnet of the 
following format:\n\txxx.xxx.xxx.xxx where xxx is between 0 and 255")
Line 904:     String thisFieldMustContainSubnetFormatAsNetmaskOrPrefixFreeMsg();


-- 
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <[email protected]>
Gerrit-Reviewer: Eliraz Levi <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to