Alon Bar-Lev has posted comments on this change. Change subject: aaa: Extensions tester tool ......................................................................
Patch Set 6: (5 comments) http://gerrit.ovirt.org/#/c/27814/6/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java: Line 39: public static final String ARG_PAGE_SIZE = "--page-size"; Line 40: public static final String ARG_IDS = "--ids"; Line 41: public static final String ARG_NAME = "--name"; Line 42: public static final String ARG_RESOLVE_GROUPS_RECURSIVE = "--resolve-groups-recursive"; Line 43: public static final String ARG_MAX_QUERY_RESULTS = "--max-query-results"; > why? in many other tools you have different parameters, what's wrong in th because the parameters needs to be more or less static. if entity adds a fields it should not cause cli interface modification. a new parameter is when something real is changing. Line 44: Line 45: // Query entity values Line 46: public static final String QUERY_ENTITY_PRINCIPAL = "principal"; Line 47: public static final String QUERY_ENTITY_GROUP = "group"; Line 238: parser.addArg(new ArgumentBuilder(). Line 239: longName(ARG_RESOLVE_GROUPS_RECURSIVE). Line 240: valueRequired(false). Line 241: build()); Line 242: > I am not against discussing future improvements here, but - let's discuss t right, just a note so you understand why the above meaningless code is [not] required. Line 243: } Line 244: return parser; Line 245: } Line 246: http://gerrit.ovirt.org/#/c/27814/6/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java: Line 18: public static void setupLogging(String log4jConfig, String logFile, String logLevel) { Line 19: URL cfgFileUrl = null; Line 20: try { Line 21: if (log4jConfig == null) { Line 22: cfgFileUrl = ExtensionsToolExecutor.class.getResource("/extensions-tool/log4j.xml"); > where do u you want me to use ths fqdn of the class, what am I missing? same issue with class conflict, see the previous patch. Line 23: } else { Line 24: cfgFileUrl = new File(log4jConfig).toURI().toURL(); Line 25: } Line 26: Log4jUtils.setupLogging(cfgFileUrl); http://gerrit.ovirt.org/#/c/27814/6/backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties File backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties: Line 1: help.usage=usage: extensions-tool <action> [<args>] > What do you mean? this is exactly how it it is done in manage domains. yes, and it was wrong.... just remove this for now. the usage (--help) should be enough. Line 2: Line 3: help.actions=\ Line 4: Available actions:\ Line 5: \n\tauth-sequence runs an authentication sequence (authenticate, fetch principal, logout)\ http://gerrit.ovirt.org/#/c/27814/6/backend/manager/pom.xml File backend/manager/pom.xml: Line 15: <module>modules</module> Line 16: <module>tools</module> Line 17: <module>extension-tool</module> Line 18: </modules> Line 19: </project> > squash what exactly? the patch you suggested does not include the tool... oh... I was confused... I read it wrong. -- To view, visit http://gerrit.ovirt.org/27814 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ea2f9c62ced5bdd3801c9f6d8087a35e3c21886 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
