mooli tayer has posted comments on this change.

Change subject: tools: Notification properties changes.
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.ovirt.org/#/c/23080/5/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java:

Line 36: /**
Line 37:  * Responsible for an execution of the service for the current events 
in the system which should be notified to the
Line 38:  * subscribers.
Line 39:  */
Line 40: public class NotificationService implements Runnable {
> Is the class name correct? For me service means that there is business logi
I think it makes sense tough its not part of my patch. I see you looked at 
change 23107 where I separated this class into a dao and left only businesses 
logic here.
Line 41: 
Line 42:     private static final Logger log = 
Logger.getLogger(NotificationService.class);
Line 43: 
Line 44:     private DataSource ds;


http://gerrit.ovirt.org/#/c/23080/5/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java
File 
backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/utils/NotificationProperties.java:

Line 148:                 if (value < 0) {
Line 149:                     throw new NumberFormatException();
Line 150:                 }
Line 151:             } catch (NumberFormatException exception) {
Line 152:                 throw new IllegalArgumentException(
> parseInt throws NumberFormatException when the string is not a number so he
I also throw it myself if value < 0 line 148
Line 153:                         String.format(
Line 154:                                 "'%s' must be a non negative 
integer.",
Line 155:                                 property));
Line 156:             }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5753ba2cad2de90a7f2d14bd4675edc1dd35045e
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Martin PeÅ™ina <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to