Skip to content

better error handling for outdated gitpython module#1847

Merged
boegel merged 7 commits intoeasybuilders:developfrom
Caylo:gitpython-error-1757
Aug 4, 2016
Merged

better error handling for outdated gitpython module#1847
boegel merged 7 commits intoeasybuilders:developfrom
Caylo:gitpython-error-1757

Conversation

@Caylo
Copy link
Copy Markdown
Contributor

@Caylo Caylo commented Jul 15, 2016

ref #1757

@boegel boegel added this to the v2.9.0 milestone Aug 3, 2016
Comment thread easybuild/tools/github.py Outdated
check_res = "OK"
elif github_user:
check_res = "FAIL (unexpected exception: %s)" % push_err
if sys.modules['git'].__version__ < "1.0.0":
Copy link
Copy Markdown
Member

@boegel boegel Aug 3, 2016

Choose a reason for hiding this comment

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

please use LooseVersion when comparing versions

also, just use git.__version__?

if LooseVersion(git.__version__) < LooseVersion('1.0'):

Did you verify whether things (should) work with GitPython 1.0 and up?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yup, verified that. Everything above 1.0 works.

Comment thread easybuild/tools/github.py Outdated
elif github_user:
check_res = "FAIL (unexpected exception: %s)" % push_err
if LooseVersion(git.__version__) < LooseVersion('1.0'):
check_res = "FAIL (outdated gitpython module, try updating first)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you clarify 'outdated' in the error, i.e. mention what the minimal version is? and also mention the what version was found

something like

check_res = "FAIL (GitPython version %s is too old, should be version %s or newer)" % (git.__version__, req_gp_ver)

Comment thread easybuild/tools/github.py Outdated
req_gp_ver = '1.0'
if LooseVersion(git.__version__) < LooseVersion(req_gp_ver):
check_res = "FAIL (GitPython version %s is too old, should be version %s or newer)" % (git.__version__,
req_gp_ver)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bleh on the wrapping here...

maybe like this?

gp_ver, req_gp_ver = git.__version__, '1.0'
if LooseVersion(gp_ver) < LooseVersion(req_gp_ver):
    check_res = "FAIL (GitPython version %s is too old, should be version %s or newer)" % (gp_ver, req_gp_ver)

@boegel
Copy link
Copy Markdown
Member

boegel commented Aug 4, 2016

Going in, thanks @Caylo!

@boegel boegel merged commit de753b3 into easybuilders:develop Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants