Yair Zaslavsky has posted comments on this change. Change subject: backend: GetoVirtISOsQuery use VersionUtils ......................................................................
Patch Set 1: (4 comments) minor comments .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java Line 85: continue; Line 86: } Line 87: Line 88: RpmVersion isoVersion = new RpmVersion(isoFileName, getOvirtIsoPrefix(), true); Line 89: isoVersion.setRpmName(isoFileName); setRpmName line is redundant. it existed in the original code, as there was a manipulation on the file name, here - there is no manipulation. Line 90: Line 91: boolean shouldAdd = false; Line 92: Line 93: String rpmParts [] = VersionUtils.splitRpmToParts(isoFileName); Line 90: Line 91: boolean shouldAdd = false; Line 92: Line 93: String rpmParts [] = VersionUtils.splitRpmToParts(isoFileName); Line 94: if (isoVersion != null && isIsoVersionSupported(rpmParts[1]) != -1) { should be isIsoVersionSupported(rpmParts[1]) - see other comments below. Line 95: if (isoData.getVdsmCompatibilityVersion() != null) { Line 96: shouldAdd = isIsoCompatibleForUpgradeByClusterVersion(isoData); Line 97: } else if (vdsOsVersion != null) { Line 98: if (VdsHandler.isIsoVersionCompatibleForUpgrade(vdsOsVersion, isoVersion)) { Line 232: } Line 233: }); Line 234: } Line 235: Line 236: private static int isIsoVersionSupported(String isoVersion) { should not be static, please check. Line 237: String supported = Config.<String> GetValue(ConfigValues.OvirtInitialSupportedIsoVersion); Line 238: return VersionUtils.compareRpmPart(isoVersion, supported); Line 239: } Line 240: Line 234: } Line 235: Line 236: private static int isIsoVersionSupported(String isoVersion) { Line 237: String supported = Config.<String> GetValue(ConfigValues.OvirtInitialSupportedIsoVersion); Line 238: return VersionUtils.compareRpmPart(isoVersion, supported); can return boolean (instead of int) should return - VersionUtils.comparerRpmPart(isoVersion,supported) >= 0; Line 239: } Line 240: Line 241: public VDS getVdsByVdsId(Guid vdsId) { Line 242: VDS vds = null; -- To view, visit http://gerrit.ovirt.org/18435 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17cae0eed9e6109fa0c7efb460e02ab7073fe21d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> 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