patternpythongitMinor
Use git to get SHA-1 of repo
Viewed 0 times
shagetgituserepo
Problem
I've created the function below to get the SHA-1 of the repository used by my CI server. The hash is then used to update the pass/fail status of the commit on GitHub.
I'm a python amateur. What stands out below?
Please advise on best practices, errors this might encounter, values I should be checking, etc.
EDIT:
Based on feedback I have revised it to the following:
I'm a python amateur. What stands out below?
def get_sha():
root = os.environ["PWD"]
repodir = os.listdir(root)[0]
# /Library/Developer/XcodeServer/Integrations/Caches/6490b1f573dca4e4e0d988197ae6c225/Source/repo_name
repo = os.path.join(root, repodir)
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shaPlease advise on best practices, errors this might encounter, values I should be checking, etc.
EDIT:
Based on feedback I have revised it to the following:
def get_sha():
for subdir in os.listdir('.'):
if is_git_directory(subdir):
return get_repo_sha(subdir)
assert False
def is_git_directory(path = '.'):
return subprocess.call(['git', '-C', path, 'status'], stderr=subprocess.STDOUT, stdout = open(os.devnull, 'w')) == 0
def get_repo_sha(repo):
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shaSolution
It seems to me that this would be equivalent but shorter:
Because,
with the difference that it's not an absolute path.
Since
I don't see the reason to use an absolute path, and so no need for
But more importantly,
I find it a bit odd that this script assumes that the first entry in the directory listing is a Git repository.
This setup seems a bit fragile.
It would be better to pass the repo path as a parameter to this function:
Then you'd have something simple, clear, no questions asked.
def get_sha():
repo = os.listdir('.')[0]
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shaBecause,
. is essentially the same as os.environ['PWD'],with the difference that it's not an absolute path.
Since
subprocess.check_output(..., cwd=...) works fine with relative paths too,I don't see the reason to use an absolute path, and so no need for
PWD.But more importantly,
I find it a bit odd that this script assumes that the first entry in the directory listing is a Git repository.
This setup seems a bit fragile.
It would be better to pass the repo path as a parameter to this function:
def get_sha(repo):
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shaThen you'd have something simple, clear, no questions asked.
Code Snippets
def get_sha():
repo = os.listdir('.')[0]
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shadef get_sha(repo):
sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
return shaContext
StackExchange Code Review Q#82630, answer score: 7
Revisions (0)
No revisions yet.