Yaniv Bronhaim has posted comments on this change.
Change subject: frontend: node upgrade message improvement
......................................................................
Patch Set 7: Code-Review-1
(6 comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostGeneralModel.java
Line 666: onPropertyChanged(new
PropertyChangedEventArgs("HasUpgradeAlert")); //$NON-NLS-1$
Line 667: }
Line 668: }
Line 669:
Line 670: public boolean shouldAlertUpgrade(ArrayList<RpmVersion> isos,
String [] hostOs)
It'll be much more readable if you'll add a comment that explains the format of
hostOs. please add.
Line 671: {
Line 672: boolean alert = false;
Line 673: Version hostVersion = new Version(hostOs[1].trim());
Line 674: String release_host = hostOs[2].trim();
Line 669:
Line 670: public boolean shouldAlertUpgrade(ArrayList<RpmVersion> isos,
String [] hostOs)
Line 671: {
Line 672: boolean alert = false;
Line 673: Version hostVersion = new Version(hostOs[1].trim());
Use RpmVersion instead of version.
Line 674: String release_host = hostOs[2].trim();
Line 675:
Line 676: for (RpmVersion iso : isos) {
Line 677: // Major check
Line 676: for (RpmVersion iso : isos) {
Line 677: // Major check
Line 678: if (hostVersion.getMajor() == iso.getMajor()) {
Line 679: // Minor and Build
Line 680: if (iso.getMinor() > hostVersion.getMinor() ||
Use RpmVersion.compareTo
Line 681: iso.getBuild() > hostVersion.getBuild()) {
Line 682: alert = true;
Line 683: break;
Line 684: }
Line 687: // Removes the ".iso" file extension , and get the
release part from it
Line 688: int isoIndex = iso.getRpmName().indexOf(".iso");
//$NON-NLS-1$
Line 689: if (isoIndex != -1) {
Line 690: rpmFromIso = iso.getRpmName().substring(0,
isoIndex);
Line 691: }
Isn't it an error not to have the .iso extension ?
Line 692: String rpmRelease =
RpmVersionUtils.splitRpmToParts(rpmFromIso)[2];
Line 693: if (RpmVersionUtils.compareRpmPart(rpmRelease,
release_host) > 0) {
Line 694: alert = true;
Line 695: break;
Line 688: int isoIndex = iso.getRpmName().indexOf(".iso");
//$NON-NLS-1$
Line 689: if (isoIndex != -1) {
Line 690: rpmFromIso = iso.getRpmName().substring(0,
isoIndex);
Line 691: }
Line 692: String rpmRelease =
RpmVersionUtils.splitRpmToParts(rpmFromIso)[2];
Redundant variable, just pass the expression to compareRpmPart
Line 693: if (RpmVersionUtils.compareRpmPart(rpmRelease,
release_host) > 0) {
Line 694: alert = true;
Line 695: break;
Line 696: }
Line 1118: ArrayList<RpmVersion> isos =
(ArrayList<RpmVersion>) returnValue;
Line 1119: if (isos.size() > 0)
Line 1120: {
Line 1121: VDS vds =
hostGeneralModel.getEntity();
Line 1122: String [] host =
vds.getHostOs().split("-"); //$NON-NLS-1$ //$NON-NLS-2$
Please change the variable name
Line 1123: hostGeneralModel.setHasUpgradeAlert(
Line 1124: shouldAlertUpgrade(
Line 1125: isos,
Line 1126: host
--
To view, visit http://gerrit.ovirt.org/17987
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76bd2948524110ec80923e855bd1f49e09e17046
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[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
