Tomas Jelinek has posted comments on this change. Change subject: frontend: refactoring: Generify list models ......................................................................
Patch Set 18: (6 comments) Only small comments - otherwise great patch! http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ListWithDetailsModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ListWithDetailsModel.java: Line 6: import org.ovirt.engine.ui.uicommonweb.models.hosts.HostInterfaceListModel; Line 7: import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs; Line 8: Line 9: @SuppressWarnings("unused") Line 10: public abstract class ListWithDetailsModel<E, D, T> extends SearchableListModel<E, T> please document in javadoc what the E, D, T is Line 11: { Line 12: Line 13: private List<HasEntity<D>> detailModels; Line 14: http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ListWithSimpleDetailsModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ListWithSimpleDetailsModel.java: Line 1: package org.ovirt.engine.ui.uicommonweb.models; Line 2: Line 3: public abstract class ListWithSimpleDetailsModel<E, T> extends ListWithDetailsModel<E, T, T> { same, please document E, T Line 4: @Override Line 5: protected T provideDetailModelEntity(T selectedItem) { Line 6: return selectedItem; Line 7: } http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListWithReportsModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListWithReportsModel.java: Line 3: import org.ovirt.engine.ui.uicompat.Event; Line 4: import org.ovirt.engine.ui.uicompat.EventArgs; Line 5: import org.ovirt.engine.ui.uicompat.EventDefinition; Line 6: Line 7: public abstract class SearchableListWithReportsModel<E, T> extends SearchableListModel<E, T> { same, please just mention what ET is Line 8: Line 9: private final Event<EventArgs> reportsAvailabilityEvent = new Event<EventArgs>(new EventDefinition("ReportsAvailabilityEvent", //$NON-NLS-1$ Line 10: SearchableListWithReportsModel.class)); Line 11: http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/pools/PoolListModel.java: Line 548 Line 549 Line 550 Line 551 Line 552 did you really want to remove this line? http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/TemplateBackupModel.java: Line 323: template.getDiskList().addAll(item.getValue()); Line 324: list.add(template); Line 325: } Line 326: Collections.sort(list, new Linq.VmTemplateComparator()); Line 327: setItems((ArrayList) list); no need to do this type cast - it is declared as ArrayList Line 328: TemplateBackupModel.this.extendedItems = items; Line 329: } Line 330: }; Line 331: GetAllFromExportDomainQueryParameters tempVar = http://gerrit.ovirt.org/#/c/32907/18/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmFromExportDomainModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ImportVmFromExportDomainModel.java: Line 260: filteredStorageDomains.add(domain); Line 261: } Line 262: } Line 263: Line 264: getStorage().setItems((ArrayList) filteredStorageDomains); why to cast? Line 265: if (hasQuota) { Line 266: initQuotaForStorageDomains(); Line 267: } else { Line 268: initDisksStorageDomainsList(); -- To view, visit http://gerrit.ovirt.org/32907 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00f1671a5839fe31b49ad0d1c99bad84e5f0c7c4 Gerrit-PatchSet: 18 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Jakub Niedermertl <jnied...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@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
