mooli tayer has posted comments on this change.

Change subject: tools: notifier - simple first match include/exclude.
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.ovirt.org/#/c/24018/4/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/filter/FirstMatchSimpleFilter.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/filter/FirstMatchSimpleFilter.java:

Line 102:                             ));
Line 103:                 }
Line 104:                 transport = s[0];
Line 105:                 name = s[1];
Line 106:             }
> this became too complex, you do not mix re and splits... I will send a diff
Yes! I see no reason not to parse recipient using the regexp.
Line 107:             ret.add(new FilterEntry(event, exclude,transport, name));
Line 108:         }
Line 109:         return Collections.unmodifiableList(ret);
Line 110:     }


Line 108:         }
Line 109:         return Collections.unmodifiableList(ret);
Line 110:     }
Line 111: 
Line 112:     public static class Recipient {
> usually types are first... not that it is important.
When I look at a class I want to see what it does so the inner classes are 
disturbing. I believe official style guides do not address this question so 
it's about being consistent.
Line 113: 
Line 114:         private final String transport;
Line 115: 
Line 116:         private final String name;


Line 162:         private final boolean exclude;
Line 163: 
Line 164:         private final Recipient recipient;
Line 165: 
Line 166:         public FilterEntry(String eventName, boolean exclude, String 
transport, String name) {
> I would have added constructor with Recipient as well :)
I am not sure why we need both. 
can't you just select one?
Line 167:             this.eventName = eventName;
Line 168:             this.exclude = exclude;
Line 169:             recipient = new Recipient(transport, name);
Line 170:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0b94ee8665ff8030b36b463207e50beea44b47d
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mta...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: mooli tayer <mta...@redhat.com>
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