Alexander Wels has posted comments on this change.
Change subject: webadmin,userportal: Upgrade GWT SDK and related modules
......................................................................
Patch Set 1: Looks good to me, but someone else must approve
(4 inline comments)
Excellent patch Vojtech, looks really good. A couple of minor nitpicks. I
noticed that most places where you refer to the GinjectorProvider which is
deprecated there is no @suppresswarning('deprecation')
Also I would try to see if we can inject some of the stuff from
GinjectorProvider into the objects if the access is not static.
About 20% of the affected files seem to be just import location changes, I
don't mind, but other people might complain.
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
Line 283: }
Line 284: }
Line 285:
Line 286: M getListModel() {
Line 287: return asEditor().flush();
Not sure how expensive asEditor().flush() if you call it more than once on an
already flushed editor. But you are calling getListModel all over the place
above, and if it is more expensive than just returning a reference, might want
to consider a local variable.
Just found the code and flush is simply returning a member variable so ignore
this. Just leaving the comment in case other people wonder.
Line 288: }
Line 289:
Line 290: @Override
Line 291: public HasEditorDriver<M> asEditor() {
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/RxTxRateColumn.java
Line 17: @Override
Line 18: protected Double[] getRawValue(T object) {
Line 19: Double rate = object != null ? getRate(object) : null;
Line 20: Double speed = object != null ? getSpeed(object) : null;
Line 21: return new Double[] { rate, speed };
shouldn't getRate and getSpeed just return null when object is null? why check
here?
Line 22: }
Line 23:
Line 24: /**
Line 25: * Returns the Rx/Tx transfer rate.
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/SideTabExtendedVirtualMachineView.java
Line 95: getTableResources(),
Line 96: ClientGinjectorProvider.getEventBus(),
Line 97: new UserPortalRefreshManager(modelProvider,
Line 98: ClientGinjectorProvider.getEventBus(),
Line 99: ClientGinjectorProvider.getClientStorage()));
These are deprecrated right? So a suppress warning on deprecated?
Line 100: }
Line 101:
Line 102: @Override
Line 103: protected Resources getTableResources() {
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/extended/vm/TooltipCell.java
Line 47: String tooltip = provider.getTooltip(value);
Line 48: if (tooltip == null) {
Line 49: tooltip = ""; //$NON-NLS-1$
Line 50: }
Line 51: // TODO(vszocs) consider using SafeHtmlTemplates instead of
building HTML manually
Is this really related to the GWT/GWTP/GIN upgrade?
Line 52: sb.appendHtmlConstant("<div id=\"" //$NON-NLS-1$
Line 53: +
ElementIdUtils.createTableCellElementId(elementIdPrefix, columnId, context)
Line 54: + "\" title=\"" //$NON-NLS-1$
Line 55: + SafeHtmlUtils.htmlEscape(tooltip)
--
To view, visit http://gerrit.ovirt.org/16739
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica797e68e2f56fd3f231baf36f0ac03c5df5c3f2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
