Tomas Jelinek has posted comments on this change. Change subject: frontend: Implementation support for GuestOsInfo and Timezone reporting ......................................................................
Patch Set 7: (6 comments) https://gerrit.ovirt.org/#/c/33376/7/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmGuestInfoModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmGuestInfoModel.java: Line 12: import org.ovirt.engine.ui.uicompat.external.StringUtils; Line 13: Line 14: public class VmGuestInfoModel extends EntityModel { Line 15: Line 16: String guestUserName; not private? Line 17: private OsType guestOsType; Line 18: private GuestOsArchitectureType guestOsArch; Line 19: private String guestOsCodename; Line 20: private String guestOsDistribution; Line 103: this.setGuestOsKernelVersion(vm.getGuestOsKernelVersion()); Line 104: this.setGuestOsType(vm.getGuestOsType()); Line 105: this.setGuestOsVersion(vm.getGuestOsVersion()); Line 106: this.setTimezoneName(vm.getTimezoneName()); Line 107: this.setTimezoneOffset(vm.getTimezoneOffset()); no need to use the "this." in any of this method Line 108: StringBuilder builder = new StringBuilder(); Line 109: if(guestOsType == OsType.Linux) { Line 110: // E.g Fedora 20 (Heisenbug) Line 111: builder.append(guestOsDistribution); Line 114: if(!StringUtils.isEmpty(guestOsCodename)) { Line 115: builder.append(" ("); //$NON-NLS-1$ Line 116: builder.append(guestOsCodename); Line 117: builder.append(')'); //$NON-NLS-1$ Line 118: } please replace this by messages (CommonApplicationMessages) - you can access them by ConstantsManager.getInstance().getMessages()... It is more readable, more fast and and also localizable. Line 119: } Line 120: else if(guestOsType == OsType.Windows && guestOs.startsWith("Win ")) { //$NON-NLS-1$ Line 121: builder.append("Microsoft Windows "); //$NON-NLS-1$ Line 122: builder.append(guestOs.substring(4)); Line 124: builder.append(" Server"); //$NON-NLS-1$ Line 125: } Line 126: builder.append(" ("); //$NON-NLS-1$ Line 127: builder.append(guestOsVersion); Line 128: builder.append(')'); //$NON-NLS-1$ same, please replace this by messages (CommonApplicationMessages) - you can access them by ConstantsManager.getInstance().getMessages()... It is more readable, more fast and and also localizable. Line 129: } Line 130: guestOsNamedVersion = builder.toString(); Line 131: Line 132: builder = new StringBuilder(); Line 140: } Line 141: builder.append(NumberFormat.getFormat("00").format(timezoneOffset / 60.)); //$NON-NLS-1$ Line 142: builder.append(':'); //$NON-NLS-1$ Line 143: builder.append(NumberFormat.getFormat("00").format(timezoneOffset % 60)); //$NON-NLS-1$ Line 144: builder.append(')'); //$NON-NLS-1$ and also this Line 145: guestOsTimezone = builder.toString(); Line 146: } Line 147: Line 148: public String getGuestOs() { https://gerrit.ovirt.org/#/c/33376/7/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java: Line 683: @DefaultStringValue("Monitor") Line 684: String monitorTitle(); Line 685: Line 686: @DefaultStringValue("Guest Information") Line 687: String sessionsTitle(); you could rename also this method Line 688: Line 689: @DefaultStringValue("RDP") Line 690: String RDPTitle(); Line 691: -- To view, visit https://gerrit.ovirt.org/33376 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia7f9ae95b2b9fe9affa27886a7981bcdffabee49 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[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
