Alexander Wels has posted comments on this change.

Change subject: frontend: refactoring: Generify Events and Listeners
......................................................................


Patch Set 8: Code-Review+1

(3 comments)

Very nice patch, cleans up a lot of stuff. A couple of super minor notes. I am 
willing to give +2 at this point, but wanted other people to review first.

http://gerrit.ovirt.org/#/c/32837/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/IProvideTickEvent.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/IProvideTickEvent.java:

Line 2: 
Line 3: import org.ovirt.engine.ui.uicompat.Event;
Line 4: import org.ovirt.engine.ui.uicompat.EventArgs;
Line 5: 
Line 6: @SuppressWarnings("unused")
I don't think this is needed here.
Line 7: public interface IProvideTickEvent
Line 8: {
Line 9:     Event<EventArgs> getTickEvent();


http://gerrit.ovirt.org/#/c/32837/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/AdElementListModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/AdElementListModel.java:

Line 322:                 if (adElementListModel.getExcludeItems() != null)
Line 323:                 {
Line 324:                     for (DbUser item : 
adElementListModel.getExcludeItems())
Line 325:                     {
Line 326:                         DbUser a = item;
You can remove this and just call

  excludeUsers.add(item.getExternalId());
Line 327: 
Line 328:                         excludeUsers.add(a.getExternalId());
Line 329:                     }
Line 330:                 }


Line 375:         Set<String> excludeUsers = new HashSet<String>();
Line 376:         if (getExcludeItems() != null) {
Line 377:             for (DbUser item : getExcludeItems()) {
Line 378:                 DbUser a = item;
Line 379:                 excludeUsers.add(a.getExternalId());
You can remove DbUser a = item; and just call:

  excludeUsers.add(item.getExternalId());
Line 380:             }
Line 381:         }
Line 382:         return excludeUsers;
Line 383:     }


-- 
To view, visit http://gerrit.ovirt.org/32837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icddf5780c00c985966e6ae956a401c3ede6a68e7
Gerrit-PatchSet: 8
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: Greg Sheremeta <gsher...@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

Reply via email to