Alon Bar-Lev has posted comments on this change. Change subject: extensions test tool: logger ......................................................................
Patch Set 21: (8 comments) https://gerrit.ovirt.org/#/c/37886/21/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ExtensionsToolExecutor.java: Line 31: private static final org.slf4j.Logger logger = LoggerFactory.getLogger(ExtensionsToolExecutor.class); Line 32: Line 33: public static void main(String[] args) { Line 34: int exitStatus = 1; Line 35: try { we need to setupLogging here, set the log level of root logger based on the value of environment variable to enable early logging. Line 36: ParametersParser parser; Line 37: try (InputStream stream = ExtensionsToolExecutor.class.getResourceAsStream("arguments.properties")) { Line 38: parser = new ParametersParser(stream, "core"); Line 39: } Line 123: modules.put(module.getName(), module); Line 124: module.setContext(context); Line 125: } Line 126: } Line 127: logger.debug("Loaded modules: {}", StringUtils.join(modules.keySet(), ",")); no need for StringUtils, modules.keySet() will work nicely :) Line 128: Line 129: return modules; Line 130: } Line 131: https://gerrit.ovirt.org/#/c/37886/21/backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/core/arguments.properties File backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/core/arguments.properties: Line 21: core.arg.version.name = version Line 22: core.arg.version.help = show version of test tool Line 23: core.help.header = oVirt Engine Extension API test tool Line 24: core.help.usage = @PROGRAM_NAME@ [options] module [options] Line 25: core.help.footer = @MODULE_LIST@ new line please https://gerrit.ovirt.org/#/c/37886/21/backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/logger/arguments.properties File backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/logger/arguments.properties: Line 24: log-record.arg.help.name = help Line 25: log-record.arg.help.help = show help for log-record action Line 26: log-record.help.header = Log-record action of logger interface test module Line 27: log-record.help.usage = @PROGRAM_NAME@ logger log-record [options] Line 28: log-record.help.footer = Example:\n @PROGRAM_NAME@ logger log-record --message=test --logger-name=TestLogger --extension-name=ovirt-logger I am almost sure you can write: log-record.help.footer = \ Example: \n\ @PROGRAM_NAME@ logger log-record --message=test --logger-name=TestLogger --extension-name=ovirt-logger please also move the --extension-name=ovirt-logger as first parameter. https://gerrit.ovirt.org/#/c/37886/21/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java File backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/cli/parser/ParametersParser.java: Line 57: public ParametersParser(InputStream resource, String prefix) { Line 58: this.properties = loadProperties(resource); Line 59: this.prefix = prefix; Line 60: parseProperties(); Line 61: } please call the other constructor. Line 62: Line 63: public Map<String, Object> parse(String... args) { Line 64: return parse(new ArrayList<String>(Arrays.asList(args))); Line 65: } Line 93: if ( Line 94: value == null && Line 95: ( Line 96: parserArgument.getType() == ArgumentType.optional_argument || Line 97: parserArgument.getType() == ArgumentType.has_argument no need to indent, unless the eclipse forces you Line 98: ) Line 99: ) { Line 100: value = args.remove(0); Line 101: if (value.equals(LONG_PREFIX)) { Line 96: parserArgument.getType() == ArgumentType.optional_argument || Line 97: parserArgument.getType() == ArgumentType.has_argument Line 98: ) Line 99: ) { Line 100: value = args.remove(0); indent? Line 101: if (value.equals(LONG_PREFIX)) { Line 102: break; Line 103: } Line 104: } Line 98: ) Line 99: ) { Line 100: value = args.remove(0); Line 101: if (value.equals(LONG_PREFIX)) { Line 102: break; why break? if at args.get(0) there is LONG_PREFIX we should keep value as null, and proceed to next step. Line 103: } Line 104: } Line 105: if (parserArgument.getType() == ArgumentType.has_argument && value == null) { Line 106: errors.add( -- To view, visit https://gerrit.ovirt.org/37886 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie06113c5d56a49e58d557c851f9ff00b9a9ca409 Gerrit-PatchSet: 21 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondřej Macháček <machacek.on...@gmail.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Ondřej Macháček <machacek.on...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org 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
