Ondřej Macháček has posted comments on this change. Change subject: aaa: extensions test tool: logger ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/37886/1/backend/manager/extension-tool/pom.xml File backend/manager/extension-tool/pom.xml: Line 39: <groupId>commons-logging</groupId> Line 40: <artifactId>commons-logging</artifactId> Line 41: <version>${commons-logging.version}</version> Line 42: <scope>compile</scope> Line 43: </dependency> > log4j please Done Line 44: </dependencies> Line 45: Line 46: <build> Line 47: <plugins> http://gerrit.ovirt.org/#/c/37886/1/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/Module.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/Module.java: Line 1: package org.ovirt.engine.exttool.core; Line 2: Line 3: public enum Module { > this class is bad as you do not want to patch it when adding new modules, t removed Line 4: Line 5: AAA("aaa"), Line 6: LOGGER("logger"); Line 7: http://gerrit.ovirt.org/#/c/37886/1/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ModuleService.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/core/ModuleService.java: Line 1: package org.ovirt.engine.exttool.core; Line 2: Line 3: public interface ModuleService { Line 4: Line 5: public void runModule(ExtensionsToolArguments args) throws Exception; > I expect at least: hmm, shouldn't be parsing args global? not per service? Line 6: http://gerrit.ovirt.org/#/c/37886/1/backend/manager/extension-tool/src/main/resources/META-INF/services/org.ovirt.engine.exttool.logger.services.LoggerService File backend/manager/extension-tool/src/main/resources/META-INF/services/org.ovirt.engine.exttool.logger.services.LoggerService: Line 1: org.ovirt.engine.exttool.logger.services.LoggerServiceImpl > please make sure you put new lines at end of file Done http://gerrit.ovirt.org/#/c/37886/1/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 655 Line 656 Line 657 Line 658 Line 659 > please sort() Done Line 1127: %{engine_data}/bin/engine-config.sh Line 1128: %{engine_data}/bin/engine-manage-domains.sh Line 1129: %{engine_data}/bin/engine-prolog.sh Line 1130: %{engine_data}/bin/ovirt-engine-role.sh Line 1131: %{engine_data}/bin/ovirt-engine-extensions-tool.sh > please sort() Done Line 1132: %{engine_data}/conf/jaas.conf Line 1133: %{engine_data}/conf/notifier-logging.properties Line 1134: %{engine_data}/conf/tools-logging.properties Line 1135: %{engine_data}/services/ovirt-engine-notifier -- To view, visit http://gerrit.ovirt.org/37886 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie06113c5d56a49e58d557c851f9ff00b9a9ca409 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondřej Macháček <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Ondřej Macháček <[email protected]> Gerrit-Reviewer: [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
