Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15537 )
Change subject: IMPALA-9107: Add timestamp to maven logging options. ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/15537/1/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/15537/1/bin/impala-config.sh@328 PS1, Line 328: export IMPALA_MAVEN_OPTIONS=${IMPALA_MAVEN_OPTIONS-} : : # If enabled, debug symbols are added to cross-comp > Maybe it's better to just to this at the same place in mvn-quiet.sh? That makes sense to me. I think that is the right place for this setting. http://gerrit.cloudera.org:8080/#/c/15537/2/bin/mvn-quiet.sh File bin/mvn-quiet.sh: http://gerrit.cloudera.org:8080/#/c/15537/2/bin/mvn-quiet.sh@34 PS2, Line 34: LOGGING_OPTIONS = -Dorg.slf4j.simpleLogger.showDateTime : DATETIME_FORMAT = -Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss You'll need to get rid of the space between the equals and the value. Might as well add quotes. (See LOG_FILE above) As a small style nit, I think I would prefer not to have two separate variables in the command below. I think of these as a single setting to enable timestamps, so it'd be great if it was one variable. This is a time when going beyond 90 characters is not so terrible. You can also wrap the line like this: LOGGING_OPTIONS="-Dorg.slf4j.simpleLogger.showDateTime \ -Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss" -- To view, visit http://gerrit.cloudera.org:8080/15537 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10fbe9eb76b66e6ba00db9f95c91063410dd1b4e Gerrit-Change-Number: 15537 Gerrit-PatchSet: 2 Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Comment-Date: Tue, 24 Mar 2020 04:22:30 +0000 Gerrit-HasComments: Yes
