Moti Asayag has posted comments on this change.
Change subject: engine: Add --first-run option to notifier
......................................................................
Patch Set 2: (5 inline comments)
....................................................
File
backend/manager/tools/engine-notifier/engine-notifier-service/src/main/java/org/ovirt/engine/core/notifier/NotificationService.java
Line 44: private Map<String, String> prop = null;
Line 45: private NotificationMethodFactoryMapper methodsMapper = null;
Line 46: private boolean shouldDeleteHistory = false;
Line 47: private int daysToKeepHistory;
Line 48:
please remove the tws
Line 49: /**
Line 50: * This flag indicates if this is the first time we are running
the notification
Line 51: * service and thus all the existing events should be marked as
processed
Line 52: * before starting.
Line 286: if (ps != null) {
Line 287: ps.close();
Line 288: }
Line 289: if (connection != null) {
Line 290: connection.close();
please use org.ovirt.engine.core.utils.db.DbUtils.closeQuietly(ps, connection)
for closing the DB resources.
Line 291: }
Line 292: }
Line 293: }
Line 294:
....................................................
File
backend/manager/tools/engine-notifier/engine-notifier-service/src/main/java/org/ovirt/engine/core/notifier/Notifier.java
Line 27: // The log:
Line 28: private static final Log log = LogFactory.getLog(Notifier.class);
Line 29:
Line 30: // Help text:
Line 31: private static final String HELP_TEXT =
why not to use the HelpFormatter to produce this message?
Line 32: "Usage: notifier [OPTIONS]... [FILE]\n" +
Line 33: "Starts the RHEV event notification service reading the\n" +
Line 34: "configuration from FILE.\n" +
Line 35: "\n" +
Line 29:
Line 30: // Help text:
Line 31: private static final String HELP_TEXT =
Line 32: "Usage: notifier [OPTIONS]... [FILE]\n" +
Line 33: "Starts the RHEV event notification service reading the\n" +
s/RHEV/ovirt ?
Line 34: "configuration from FILE.\n" +
Line 35: "\n" +
Line 36: "Available options:\n" +
Line 37: " -h, --help Display this help text and exit.\n" +
Line 139: System.exit(0);
Line 140: }
Line 141:
Line 142: // Get the values of the options:
Line 143: firstRun = line.hasOption("first-run");
maybe worth extracting the names of the command line options into constants
since repeated (although it is only twice, excluding the helper-text, it seems
that having the definition in a single attribute is less error-prone in respect
to maintaining it)
Line 144:
Line 145: // Get the name of the configuration file:
Line 146: args = line.getArgs();
Line 147: if (args != null && args.length == 1) {
--
To view, visit http://gerrit.ovirt.org/8772
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I242bc0fbeab73d7f2d363b3e9ebf895af0131033
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
