Alexander Wels has posted comments on this change. Change subject: engine, webadmin: Webadmin read reports.xml from remote reports app ......................................................................
Patch Set 25: (4 comments) A few minor issues, but looks fine to me otherwise. http://gerrit.ovirt.org/#/c/29723/25/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabClusterView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabClusterView.java: Line 35: interface ViewIdHandler extends ElementIdHandler<MainTabClusterView> { Line 36: ViewIdHandler idHandler = GWT.create(ViewIdHandler.class); Line 37: } Line 38: Line 39: private ApplicationConstants constants; Why have this? To me it looks like the only reason is so it can be used in updateReportsAvailability(). But that is called from a method that already has the contants parameter, so why not just pass the contants variable to the updateReportsAvailability() method? Line 40: Line 41: @Inject Line 42: public MainTabClusterView(MainModelProvider<VDSGroup, ClusterListModel> modelProvider, Line 43: ApplicationResources resources, ApplicationConstants constants) { http://gerrit.ovirt.org/#/c/29723/25/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabDataCenterView.java: Line 37: interface ViewIdHandler extends ElementIdHandler<MainTabDataCenterView> { Line 38: ViewIdHandler idHandler = GWT.create(ViewIdHandler.class); Line 39: } Line 40: Line 41: ApplicationConstants constants; Why have this? To me it looks like the only reason is so it can be used in updateReportsAvailability(). But that is called from a method that already has the contants parameter, so why not just pass the contants variable to the updateReportsAvailability() method? Line 42: Line 43: @Inject Line 44: public MainTabDataCenterView(MainModelProvider<StoragePool, DataCenterListModel> modelProvider, Line 45: ApplicationResources resources, ApplicationConstants constants) { http://gerrit.ovirt.org/#/c/29723/25/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabStorageView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabStorageView.java: Line 38: interface ViewIdHandler extends ElementIdHandler<MainTabStorageView> { Line 39: ViewIdHandler idHandler = GWT.create(ViewIdHandler.class); Line 40: } Line 41: Line 42: ApplicationConstants constants; Why have this? To me it looks like the only reason is so it can be used in updateReportsAvailability(). But that is called from a method that already has the contants parameter, so why not just pass the contants variable to the updateReportsAvailability() method? Line 43: Line 44: @Inject Line 45: public MainTabStorageView(MainModelProvider<StorageDomain, StorageListModel> modelProvider, Line 46: ApplicationConstants constants) { http://gerrit.ovirt.org/#/c/29723/25/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabVirtualMachineView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabVirtualMachineView.java: Line 51: ViewIdHandler idHandler = GWT.create(ViewIdHandler.class); Line 52: } Line 53: Line 54: private final CommonApplicationConstants commonConstants; Line 55: private final ApplicationConstants constants; Why have this? To me it looks like the only reason is so it can be used in updateReportsAvailability(). But that is called from a method that already has the contants parameter, so why not just pass the contants variable to the updateReportsAvailability() method? Line 56: Line 57: @Inject Line 58: public MainTabVirtualMachineView(MainModelProvider<VM, VmListModel> modelProvider, Line 59: ApplicationResources resources, ApplicationConstants constants, -- To view, visit http://gerrit.ovirt.org/29723 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76db7ab889f21de083bb3c8276e8abb77b68fdb3 Gerrit-PatchSet: 25 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Shirly Radco <sra...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
