Lars Volker has posted comments on this change.

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


Patch Set 1:

(3 comments)

Thanks for fixing this.

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

PS1, Line 34: can_obtain_git_hash = False if os.system('git rev-parse HEAD') 
else True
This took me a second glance to understand, maybe replace with

can_obtain_git_hash = os.system(...) == 0 ?

Also, discard stdout and stderr of the git command?


PS1, Line 39: if not version_file_exists or can_obtain_git_hash:
what happens if we cannot obtain the git hash and don't have a version file? 
Maybe explain here that it is intended to call SAVE_VERSION_SCRIPT anyways in 
this case. You could also reverse the if and else clauses and make the if 
condition's structure the comment. Maybe that would make it easier to 
understand.


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

Line 25: GIT_HASH=$(git rev-parse HEAD)
Discard stderr?


-- 
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: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to