Martin Betak has posted comments on this change. Change subject: frontend: refactoring: Generify list models ......................................................................
Patch Set 25: (1 comment) https://gerrit.ovirt.org/#/c/32907/25/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java: Line 288: Line 289: entityModel = new EntityModel<E>() { Line 290: @Override Line 291: protected void onEntityChanged() { Line 292: super.onEntityChanged(); > I'm almost sure these calls will cause regression. It means that when you o Yes this empty method will always be called. I wanted to put the calls there for consitency. 'onEntityChanged' and 'entityPropertyChanged' are empty in EntityModel so this breaks nothing. The only non-empty method from these 3 is the 'entityChanging' which has the following 2 inheritors overriding it: ClusterNetworkListModel and EventListModel. Of these 2 only ClusterNetworkListModel does not call super and I believe this omission is unintentional. I believe the EntityModel's implementation should always be called. Subclasess should only react to those changes and not interfere in the implementation of EntityModel. Lior: since ClusterNetworkListModel seems like your are, can you confirm that ClusterNetworkListModel.entityChanging should call super? Line 293: SearchableListModel.this.onEntityChanged(); Line 294: } Line 295: Line 296: @Override -- To view, visit https://gerrit.ovirt.org/32907 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00f1671a5839fe31b49ad0d1c99bad84e5f0c7c4 Gerrit-PatchSet: 25 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
