Alexey Serbin has posted comments on this change. Change subject: WIP [jepsen.kudu] run tests from clojure-maven-plugin ......................................................................
Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/5551/7/java/kudu-jepsen/.gitignore File java/kudu-jepsen/.gitignore: Line 1: .lein-failures > Could you add comments explaining why we need each one? Done http://gerrit.cloudera.org:8080/#/c/5551/7/java/kudu-jepsen/README.md File java/kudu-jepsen/README.md: Line 22: List of required packages For Debian/Ubuntu machines if us: > Could you reformat this sentence? It has some grammar issues and multiple c Done PS7, Line 23: krb5-admin-server krb5-kdc krb5-user : libsasl2-modules libsasl2-modules-gssapi-mit lsb-release ntp openssl > Nit: reformat this into an actual markdown list. Done. Actually, I automated that, so this comment and corresponding monkey work is not necessary anymore. PS7, Line 27: on > of Done http://gerrit.cloudera.org:8080/#/c/5551/7/java/kudu-jepsen/pom.xml File java/kudu-jepsen/pom.xml: PS7, Line 29: <masterNodes></masterNodes> : <tserverNodes></tserverNodes> : <sshKeyPath></sshKeyPath> > Why is this necessary? Aren't all properties empty by default? I thought it's better to explicitly declare those. Besides, as it turned out, properties are not empty by default. At least, when the property is not listed here and used like in <args> element below for clojure-maven-plugin, it is evaluated to "${sshKeyPath}" (!sic). Since "${sshKeyPath}" is not "", that breaks the test. I'm going to leave them as is. If you know the better way to deal with that -- let me know. Line 102: <args>--master-nodes=${masterNodes} --tserver-nodes=${tserverNodes} --ssh-key-path=${sshKeyPath}</args> > Nit: could you separate each argument on its own line so this is more reada Done -- To view, visit http://gerrit.cloudera.org:8080/5551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5adb6968df46954f94c11f29ecc4dd4ea56544b1 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
