Lior Vernia has posted comments on this change. Change subject: webadmin: removed ugly if-else chain ......................................................................
Patch Set 1: (1 comment) Good patch, just one inline comment. And also, could you please rebase so that this comes before the preceding patch, and then drop the change to AsyncDataProvider in that patch? http://gerrit.ovirt.org/#/c/33619/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java: Line 3224: public Guid getEntityGuid(Object entity) { Line 3225: Line 3226: if (entity instanceof BusinessEntity) { Line 3227: //BusinessEntity can have lot of different ID types, but from this context it cannot be determined. Line 3228: Object id = getEntityGuid((BusinessEntity<?>) entity); Why is this method needed? You're already casting the entity - might as well inline the getId(). There's no benefit in binding the generic parameter to T either... Line 3229: Line 3230: //check whether result can be casted to Guid, otherwise continue with explicit rules. Line 3231: if (id instanceof Guid) { Line 3232: return (Guid) id; -- To view, visit http://gerrit.ovirt.org/33619 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic716dc67b90c1d575de14c40b504c984bf0467f2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
