Todd Lipcon has posted comments on this change. Change subject: IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs. ......................................................................
Patch Set 1: (5 comments) looks pretty reasonable. Might want to do a surface level understanding of why you need this ratcursive workaround and file a JIRA upstream. http://gerrit.cloudera.org:8080/#/c/4361/1/bin/rat.sh File bin/rat.sh: PS1, Line 38: RAT_JAR=$1 : # The location of the Impala directory (or subdirectory within the Impala tree) to check: : START=$2 rather than relying just on set -u, i think it's nicer to print a real 'usage: $0 ...' PS1, Line 51: PN what's PN? Line 54: if (ulimit -t 10; java -jar "${RAT_JAR}" -x "${FN}" >"${OUTFILE}"); then wow, neat trick I'd never seen Line 58: if [ -d "${FN}" ]; then maybe you want to avoid recursing into symlinks? Line 63: echo "Failed to RAT ${FN}" >&2 should set the exit code in this case, no? otherwise people might miss this. -- To view, visit http://gerrit.cloudera.org:8080/4361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5bfe77f9a871018e7a67553ed270e2df53006962 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
