Mike Kolesnik has posted comments on this change. Change subject: core: few fixes in MacAddressRangeUtils.java ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/26404/6/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java: Line 21: long startNum = macStringToLong(start); Line 22: long endNum = macStringToLong(end); Line 23: Line 24: return innerInitRange(size, startNum, endNum); Line 25: } catch (InvalidMacString e) { > to preserve same behavior as there already was. This is just moved method ( Don't see anywhere in the original code such behavior, also the exception was added by you so not sure what you mean by your reply. Line 26: LOGGER.warn(e); Line 27: return Collections.emptyList(); Line 28: } Line 29: } Line 73: Line 74: if (startEndArray.length == 2) { Line 75: rwo.addRange(macStringToLong(startEndArray[0]), macStringToLong(startEndArray[1])); Line 76: } else { Line 77: LOGGER.errorFormat("Failed to initialize Mac Pool range. Please fix Mac Pool range: {0}", rangesArray[i]); > it's refactor, not rewrite ~~ not mine code, not mine decision. This was i Doesn't seem like it moved from anywhere, I see it now in MacPoolManager but from this patch alone this is unclear. Line 78: } Line 79: } Line 80: Line 81: return rwo.getRanges(); Line 105: Line 106: try { Line 107: String parsedRangeStart = StringUtils.remove(mac, ':'); Line 108: return Long.parseLong(parsedRangeStart, HEX_RADIX); Line 109: } catch (NumberFormatException | NullPointerException e) { > original code catches Exception. If former code did not fail, neither this Don't see anywhere in the original code that it's catching these exceptions. Line 110: throw new InvalidMacString(mac, e); Line 111: } Line 112: } Line 113: Line 117: long startNum = macStringToLong(start); Line 118: long endNum = macStringToLong(end); Line 119: Line 120: result = innerInitRange(1, startNum, endNum); Line 121: } catch (InvalidMacString e) { > former class was lenient, I cannot change it. This is an exception you added, so not sure what the leniency has got to do with it.. Also former implementation didn't catch any exception so if there was one it would propagate to the caller. Line 122: LOGGER.warn(e); Line 123: result = Collections.emptyList(); Line 124: } Line 125: return !result.isEmpty(); -- To view, visit http://gerrit.ovirt.org/26404 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I098ade5fc3854ad23236abceebb87cc51908c65f Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[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
