Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11274 )
Change subject: [hms] update the HIVE/HADOOP package scripts ...................................................................... Patch Set 1: (12 comments) http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10 PS1, Line 10: chose choose http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10 PS1, Line 10: repackaing repackaging http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh File thirdparty/package-hadoop.sh: PS1: Pretty much all of the feedback here applies to package-hive.sh too. http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@26 PS1, Line 26: Optional Nit: optional (lower case). On L28 too. http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@39 PS1, Line 39: Dowdload Download http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@40 PS1, Line 40: -repackage --repackage http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@49 PS1, Line 49: if [ ${OPTS_RESULT} != 0 ] ; then echo "Failed parsing options." >&2 ; exit ${OPTS_RESULT} ; fi Nit: break this up onto separate lines. http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@51 PS1, Line 51: do Nit: separate with a space from the semicolon http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@53 PS1, Line 53: echo here This was just for debugging, right? http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@55 PS1, Line 55: -h | --help ) Nit: -h | --help) (to the other cases here too) http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@69 PS1, Line 69: echo here Remove this? http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@74 PS1, Line 74: for DIR in client common/jdiff common/sources hdfs/sources httpfs kms mapreduce/lib-examples mapreduce/sources tools; do Can you construct this list more clearly? Like: DIRS="client" DIRS="$DIRS common/jdiff" DIRS="$DIRS common/sources" ... for DIR in $DIRS; do ... done While more verbose, this format makes it easier to find individual entries and to keep them sorted. -- To view, visit http://gerrit.cloudera.org:8080/11274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 Gerrit-Change-Number: 11274 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:50:35 +0000 Gerrit-HasComments: Yes
