Tomas Jelinek has posted comments on this change.

Change subject: Mark VM as problematic when timezone or OS changed
......................................................................


Patch Set 8:

(7 comments)

https://gerrit.ovirt.org/#/c/30508/8/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/TimeZoneType.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/TimeZoneType.java:

Line 223:         String s = getTimeZoneList().get(timeZoneKey);
Line 224:         Match match = Regex.Match(s, TimeZoneExtractTimePattern);
Line 225:         int value = 0;
Line 226:         if(match.success() && match.groups().size() > 0) {
Line 227:             value = 
Integer.parseInt(match.groups().get(0).getValue().substring(3).replace(":", 
"").replace("+", ""));
this complex expression is copy pasted 3 times already now - please extract to 
a standalone method.
Line 228:             boolean neg = value < 0;
Line 229:             if(neg) {
Line 230:                 value *= -1;
Line 231:             }


Line 224:         Match match = Regex.Match(s, TimeZoneExtractTimePattern);
Line 225:         int value = 0;
Line 226:         if(match.success() && match.groups().size() > 0) {
Line 227:             value = 
Integer.parseInt(match.groups().get(0).getValue().substring(3).replace(":", 
"").replace("+", ""));
Line 228:             boolean neg = value < 0;
instead of this you could do "value = Math.abs(value);"
Line 229:             if(neg) {
Line 230:                 value *= -1;
Line 231:             }
Line 232:             value = ((value / 100) * 60 + value % 100);


Line 230:                 value *= -1;
Line 231:             }
Line 232:             value = ((value / 100) * 60 + value % 100);
Line 233:             if(neg) {
Line 234:                 value *= -1;
here it can not be negative anymore
Line 235:             }
Line 236:         }
Line 237:         return value;
Line 238:     }


https://gerrit.ovirt.org/#/c/30508/8/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/VmStatusCell.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/VmStatusCell.java:

Line 139:             String html = imagePrototype.getHTML();
Line 140: 
Line 141:             // Append tooltip
Line 142:             EnumTranslator translator = EnumTranslator.getInstance();
Line 143:             String toolTip = "";
miss the //$NON-NLS-1$
Line 144:             if(updateNeeded) {
Line 145:                toolTip = constants.newtools();
Line 146:             } else if(vm.getStatus() != VMStatus.Up) {
Line 147:                toolTip = translator.translate(vm.getVmPauseStatus());


Line 149:             if(timezoneDiffers) {
Line 150:                 if(toolTip.length() > 0) {
Line 151:                     toolTip += " | "; //$NON-NLS-1$
Line 152:                 }
Line 153:                 constants.guestTimezoneDiffers();
tooltip += ... :)
Line 154:             }
Line 155:             if(osTypeDiffers) {
Line 156:                 if(toolTip.length() > 0) {
Line 157:                     toolTip += " | "; //$NON-NLS-1$


Line 155:             if(osTypeDiffers) {
Line 156:                 if(toolTip.length() > 0) {
Line 157:                     toolTip += " | "; //$NON-NLS-1$
Line 158:                 }
Line 159:                 toolTip += " | " + constants.guestOSDiffers(); 
//$NON-NLS-1$
you put the "|" twice here
Line 160:             }
Line 161: 
Line 162:             html = html.replaceFirst("img", "img " + "title='" + 
toolTip + "' "); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
Line 163: 


Line 185:             if (javaZoneId != null) {
Line 186:                 offset = 
TimeZoneType.GENERAL_TIMEZONE.getStandardOffset(javaZoneId);
Line 187:             }
Line 188:         }
Line 189:         if(vm.getTimezoneOffset() != offset) {
if the timeZone == null (or empty) than the offset is going to stay 0. 

In this case this method will return true or false depending on the 
vm.getTimezoneOffset. 
But it is not correct because the offset is not actually 0, it is just unknown
Line 190:             return true;
Line 191:         }
Line 192: 
Line 193:         return false;


-- 
To view, visit https://gerrit.ovirt.org/30508
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1d6c878a3e700998c77d3ee3248b4ef3302d08b
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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

Reply via email to