Yair Zaslavsky has posted comments on this change.

Change subject: frontend:  node upgrade message improvement
......................................................................


Patch Set 7:

(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)
I totally agree.
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());
HostOs format is unfortunately NOT in RpmVersion format, and VDSM side does not 
calculate it using RpmName.
The 2nd part is in the format of Version, unfortunately.
No real benefit from switching to RpmVersion here, not to mention I cannot do 
that, as this is GWT code, no regex.
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() ||
I can't. I am not interesting in checking all the parts , and i need to 
distinguish the case where major is equal.
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:                 }
Why?
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];
Matter of taste, I won't argue here :)
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$
Which one to change? please suggest alternative.
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 <dougsl...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@gmail.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@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

Reply via email to