-
Notifications
You must be signed in to change notification settings - Fork 227
Use enum34 backport when Enum is not available #177
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.
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
Use enum34 backport when Enum is not available #177
Conversation
2 similar comments
I also fixed :
|
6a54188
to
1c2bdb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my inline comment in the last commit. Otherwise looks fine.
I added a commit fixing build issues, but they are mainly related to a wrong requirements management in setup.py and broken dependancies in graphql-core All these things should be fixed propelry soon by work from @jnak on tox migration and from a PR on graphene / graphql-core |
@Nabellaleen I'm almost done with the migration to tox. I'll be sending a PR tomorrow because I'm off today. |
@jnak have a look at the change I made in this PR to take it into account in yours (but it's probably already the case ;)) |
@Nabellaleen Sounds good. I'll take a look tomorrow morning |
setup.py
Outdated
"graphene>=2.1.3", | ||
"SQLAlchemy", | ||
"singledispatch>=3.4.0.3", | ||
"rx<3.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"rx<3.0.0", | |
"rx<3.0.0", # Rx 3.0 or above are only compatible with Python >= 3.6 |
@Nabellaleen In case you're curious, here are the changes I worked on yesterday. In order to avoid duplicating work, I think it makes sense to hold on this PR for now and wait for my changes to be merged in. I'll be sending a PR tomorrow. Cheers |
@jnak Yeah, I see ;) Great job ! I can't wait to see things move :D But I'll make an effort and be patient ;) |
Co-Authored-By: Nabellaleen <Nabellaleen@users.noreply.github.com>
a0afe0e
to
2db8fef
Compare
@Nabellaleen Thanks for re-enabling python 2.7. Out of curiosity, what's the policy to merge PRs in this project? It looks like you were able to merge this PR without getting it approved by someone else. |
@jnak Indeed, rules seems not enforced in the repository settings - I'll fix it ! For the graphene organisation, the rules were defined some weeks ago during a meeting, and are specific for each repository (between 1 and 3 approvals for a merge). For graphene-sqlalchemy repository, it's 1 approval, so I should have been blocked for merging this one. |
Tests were broken in Python < 3.4, this PR fix its