Martin Mucha has posted comments on this change. Change subject: core: lenient validation of mac pool ranges in engine-config ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/35390/1/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java File backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java: Line 41 Line 42 Line 43 Line 44 Line 45 > is this validation removed due to the nature of LongRange c'tor which arran yes. I think it's not worth it enforcing this, if implementation does not care about the order. I can be a little messy though, would you rather see strict syntax? Line 76: return new ValidationResult(true); Line 77: } Line 78: Line 79: private ValidationResult validateRangePart(String rangePart) { Line 80: rangePart.toLowerCase(); > no assignment ?? is there any importance to the case sensitivity? crap, sorry about that assignment. this lowering is probably pointless since matcher does not care, was only used(line 33, 34) there to print error message in lowercase. I opted to preserve that. Do you think I can remove that entirely? a) remove b)fix assignment and keep MAC address in error message in lowercase. Line 81: boolean matches = MAC_ADDRESS_PATTERN.matcher(rangePart).matches(); Line 82: if (!matches) { Line 83: return new ValidationResult(false, "The range start/end is an invalid MAC address. " + rangePart Line 84: + " should be in a format of AA:AA:AA:AA:AA:AA"); -- To view, visit http://gerrit.ovirt.org/35390 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7e47c1ab1c2ffd340e1ff2596ee567557324a28 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[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
