Alon Bar-Lev has posted comments on this change. Change subject: extensions test tool: logger ......................................................................
Patch Set 15: (3 comments) https://gerrit.ovirt.org/#/c/37886/15/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 51: moduleService.parseArguments(others); Line 52: loadExtensions(moduleService, argMap); Line 53: moduleService.run(); Line 54: } catch(ExitException e) { Line 55: if(e.isHelpPrint()) { help cannot be part of exit exception, help should be printed within logic. if you do want it as exception then setup own exception or that, but I am unsure this is correct approach. Line 56: List<String> modules = new ArrayList<>(moduleServices.keySet()); Line 57: Collections.sort(modules); Line 58: System.out.println( Line 59: String.format( https://gerrit.ovirt.org/#/c/37886/15/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 20: __single__.arg.help.help = show test tool help Line 21: __single__.arg.version.name = version Line 22: __single__.arg.version.help = show version of test tool Line 23: help.underlinetext = You can use this tool to test your extension. Line 24: help.usage = module help should also be prefixed https://gerrit.ovirt.org/#/c/37886/15/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: usage = "__single__"; Line 58: } else { Line 59: usage = args.remove(0); Line 60: argMap.put("__action__", usage); Line 61: } I still do not understand why the parser should address the action... I claim that this is multi level execution of same parser. it should not address anything special just know how to parse, and we call it multiple times to parse nested. the output of usage can be nested as well so --help on base issues one set of usage and --help on action returns a different set (only action) usage. if we want a complete usage, we can register usage of each action within core, so core can print all, again this does not mean the parser is aware of this multi level parsing. Line 62: if(usage.equals("--help") || (!usage.equals("__single__") && !args.isEmpty() && args.contains("--help"))) { Line 63: throw new ExitException(getHelp(), 0, true); Line 64: } Line 65: -- 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: 15 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
