Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8102 )
Change subject: IMPALA-5376: Implement all TPCDS test cases or alternates for Impala. ...................................................................... Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG@11 PS5, Line 11: $IMPALA_HOME/bin/buildall.sh Nit: buildall.sh is in the root of Impala, not bin/ http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG@12 PS5, Line 12: This commit continues previous work on IMPALA-5376 under the apache/incubator-impala repo : on github.com, and commit 6877 at gerrit.cloudera.org:8080. Not sure this is needed. You reference IMPALA-5376 in the commit title appropriately. Gerrit, github, etc. are all transient. I don't expect you'll find many of any mentions of them in commit messages. I suggest you just delete this paragraph. You could rephrase to mention continuing previous work if you wish, but to me, the whole paragraph can be removed. http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG@15 PS5, Line 15: runs with passes Can you explain in the commit message how you made the decision to have passing tests? In other words, what served as the source of truth such that you have confidence you can say these tests are passing? http://gerrit.cloudera.org:8080/#/c/8102/5//COMMIT_MSG@17 PS5, Line 17: Names for : such files have -1, -2... inner suffixes. Can you file a follow on Jira for the stress test (project: IMPALA; component: Infrastructure) to support recognizing test files with suffixes? I don't think it does today. http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q23.test File testdata/workloads/tpcds/queries/tpcds-q23.test: http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q23.test@51 PS1, Line 51: : : > The stress test gets its bank of queries to run from files such as this, bu Done http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q24.test File testdata/workloads/tpcds/queries/tpcds-q24.test: http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q24.test@48 PS1, Line 48: : : : : > Highlighting another place where there are 2 queries and the stress test's Done http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q39.test File testdata/workloads/tpcds/queries/tpcds-q39.test: http://gerrit.cloudera.org:8080/#/c/8102/1/testdata/workloads/tpcds/queries/tpcds-q39.test@27 PS1, Line 27: : > Another multi-query file, like previously. Done -- To view, visit http://gerrit.cloudera.org:8080/8102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e284888600a7a69d1f23fcb7dac21cbb13b7d66 Gerrit-Change-Number: 8102 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Wood <tw...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Matthew Mulder <mmul...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Tim Wood <tw...@cloudera.com> Gerrit-Comment-Date: Fri, 22 Sep 2017 20:45:06 +0000 Gerrit-HasComments: Yes
