Nithya Janarthanan has posted comments on this change. ( http://gerrit.cloudera.org:8080/9273 )
Change subject: IMPALA-5139: Update mvn-quiet.sh to print execution content to log file ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/9273/5/bin/mvn-quiet.sh File bin/mvn-quiet.sh: http://gerrit.cloudera.org:8080/#/c/9273/5/bin/mvn-quiet.sh@25 PS5, Line 25: $IMPALA_MVN_LOGS_DIR > This should be quoted in case a parent directory has a space. Sure...will do http://gerrit.cloudera.org:8080/#/c/9273/5/bin/mvn-quiet.sh@27 PS5, Line 27: #Print the output of mvn commands to a dedicated log file in $IMPALA_HOME/logs/mvn/mvn.log > I'm not sure this comment is needed. Typically comments should say why is s ok http://gerrit.cloudera.org:8080/#/c/9273/5/bin/mvn-quiet.sh@29 PS5, Line 29: echo -e " ========================================================================\ : \n Running mvn $IMPALA_MAVEN_OPTIONS $@ \ : \n Directory: $(pwd) \ : \n ========================================================================" >> $LOG_FILE > I find this to be a little cluttered, with the backslashes and the \n at th Sure...will do http://gerrit.cloudera.org:8080/#/c/9273/5/bin/mvn-quiet.sh@35 PS5, Line 35: mvn "$@" | tee -a $LOG_FILE | grep -E -e WARNING -e ERROR -e SUCCESS -e FAILURE -e Test; then > > * I still think mvn being piped to mvn is an odd construction, and probab Working on the review comments -- To view, visit http://gerrit.cloudera.org:8080/9273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I475b17a4dccfa624dda61402491b461c53473f8b Gerrit-Change-Number: 9273 Gerrit-PatchSet: 5 Gerrit-Owner: Nithya Janarthanan <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Nithya Janarthanan <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Thu, 15 Feb 2018 19:28:47 +0000 Gerrit-HasComments: Yes
