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

fix: need gevent >= 1.2, not > 1.1 #483

Merged
merged 1 commit into from
Nov 4, 2017
Merged

fix: need gevent >= 1.2, not > 1.1 #483

merged 1 commit into from
Nov 4, 2017

Conversation

jeffwidman
Copy link
Member

Python 2.6 support was dropped in 1.2, so 2e8dcd3 simplified the dependency graph to require gevent>1.1. However, bugfix releases to the 1.1 series (1.1.1, etc) will satisfy this requirement, so we should actually be requiring gevent >= 1.2

jeffwidman added a commit that referenced this pull request Nov 3, 2017
@jeffwidman
docs: Correctly document supported versions
f860de5 
Gevent support < 1.2 was dropped in 2e8dcd3 (and further tweaked in #483).

Travis already tests the Zookeeper 3.5 series.
@jeffwidman jeffwidman mentioned this pull request Nov 3, 2017
Merged
hannosch
hannosch previously approved these changes Nov 3, 2017
View reviewed changes
@jeffwidman
Copy link
Member Author

@hannosch do you mind approving again? I merged #484 first, and then Github complained that i needed to merge that into this first, but when I solved that by simply rebasing, Github dismissed your review... The change is the exact same, simply rebased.

@hannosch hannosch self-requested a review November 3, 2017 18:39
hannosch
hannosch previously approved these changes Nov 3, 2017
View reviewed changes
@jeffwidman
fix: need gevent >= 1.2, not > 1.1
18f3531 
Python 2.6 support was dropped in 1.2, so 2e8dcd3 simplified the dependency graph to require `gevent>1.1`. However, bugfix releases to the 1.1 series (1.1.1, etc) will satisfy this requirement, so we should actually be requiring `gevent >= 1.2`

Additionally, these requirements should be specified in setup.py as
well.
@jeffwidman
Copy link
Member Author

jeffwidman commented Nov 3, 2017 •
edited
Loading

@hannosch sorry, do you mind reviewing one more time? I realized right after my last comment that we weren't applying this minimum version pinning in setup.py. I just pushed a fix on that (which again unfortunately dismissed your review again)

@jeffwidman jeffwidman merged commit f1ccd1e into master Nov 4, 2017
@jeffwidman jeffwidman deleted the fix-gevent-requirement branch November 4, 2017 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannosch hannosch hannosch approved these changes

@bbangert bbangert Awaiting requested review from bbangert

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeffwidman @hannosch