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