Michael Brown has posted comments on this change.

Change subject: Make gen_build_version.py resilient to a failing git rev-parse
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4411/4/bin/gen_build_version.py
File bin/gen_build_version.py:

PS4, Line 36: with open(os.devnull, 'w') as devnull:
Does this Python script need to support ancient Python 2.4? If yes, with won't 
be supported and this will fail.


http://gerrit.cloudera.org:8080/#/c/4411/4/bin/save-version.sh
File bin/save-version.sh:

PS4, Line 25: GIT_HASH=$(git rev-parse HEAD) 2> /dev/null
"2> /dev/null" should go inside the parens with the command.


PS4, Line 28:   GIT_HASH="Could not obtain git hash"
Has this path been tested? Does Impala work when the derived git hash doesn't 
match the typical pattern of a git hash? It might be safer to echo a warning 
here and make GIT_HASH empty. We know an empty GIT_HASH already won't cause 
Impala to crash or exit. If the point here is "best effort", this seems safer.


-- 
To view, visit http://gerrit.cloudera.org:8080/4411
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7af33502bbb70185dc15ffca6219436a616f25b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to