Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Silently omit git argument when git is unavailable #176

Merged
merged 3 commits into from
Feb 26, 2018
Merged

Silently omit git argument when git is unavailable #176

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d97137
feat(git): Silently omit git if unavailable
nirizr Feb 22, 2018
411633d
feat(git): Add warning if git is unavailable
nirizr Feb 23, 2018
b544f2b
feat(git): use actual commands, include exception
nirizr Feb 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions coveralls/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ def git_info():
}]
}
"""
# silently omit the optional 'git' API argument if a simple git status
# command fails, suggesting git is unavailable
try:
run_command('git', 'status')
except CoverallsException:
return {}

rev = run_command('git', 'rev-parse', '--abbrev-ref', 'HEAD').strip()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please put the try catch around the already-existing commands? No need to run an extra git command here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that; the original reason I ended up adding a git status command was to avoid any possible errors related specifically to the existing commands, that may be because of something other than git not being available.

I can't think of a good scenario but an unlikely example could be having a git version that does not support the current command (or any future changes). Either because git version is too old or too new with backwards incompatible cli changes.

WDYT?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense -- I think if we add the logging, something like "could not collect git data -- are you running coveralls in a repository?", along with logging the CoverallsException message (which contains the process output), that should cover it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Will amend with a more verbose log (and phrasing that's more to your liking).

remotes = run_command('git', 'remote', '-v').splitlines()

Expand Down