Alexander Wels has posted comments on this change.

Change subject: webadmin: Improve UI Plugin tab API
......................................................................


Patch Set 2: Looks good to me, but someone else must approve

(2 inline comments)

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractSubTabPresenter.java
Line 115:         // Notify model provider that the tab has been revealed
Line 116:         modelProvider.onSubTabSelected();
Line 117: 
Line 118:         if (getTable() != null) {
Line 119:             getTable().setLoadingState(LoadingState.LOADING);
I don't know how expensive a 'getTable()' is, but if you are going to call it 
more than once in a method would it make sense to store it in an intermediate 
variable?
Line 120:         }
Line 121:     }
Line 122: 
Line 123:     @Override


....................................................
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/cluster/SubTabClusterPolicyPresenter.java
Line 44:             ViewDef view,
Line 45:             ProxyDef proxy,
Line 46:             PlaceManager placeManager,
Line 47:             DetailModelProvider<ClusterListModel, ClusterPolicyModel> 
modelProvider,
Line 48:             ApplicationConstants constants) {
Do we need the application constants injected if we don't use it anywhere? For 
that matter if we don't need it injected, do we even need this constructor? all 
it would do is just call super.
Line 49:         super(eventBus, view, proxy, placeManager, modelProvider);
Line 50:     }
Line 51: 
Line 52:     @Override


-- 
To view, visit http://gerrit.ovirt.org/15810
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f3e33844693a08352cc54b60b05b3f6ac5eb816
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Keith Robertson <krobe...@redhat.com>
Gerrit-Reviewer: Spenser Shumaker <sshum...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to