Skip to content

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

Merged

Conversation

Nabellaleen
Copy link
Collaborator

Tests were broken in Python < 3.4, this PR fix its

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.877% when pulling f8b3962 on Nabellaleen:fix-enum-backport-requirement into 6a96d37 on graphql-python:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.877% when pulling f8b3962 on Nabellaleen:fix-enum-backport-requirement into 6a96d37 on graphql-python:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.877% when pulling f8b3962 on Nabellaleen:fix-enum-backport-requirement into 6a96d37 on graphql-python:master.

@coveralls
Copy link

coveralls commented Nov 26, 2018

Coverage Status

Coverage remained the same at 91.877% when pulling a61580a on Nabellaleen:fix-enum-backport-requirement into 8a2f020 on graphql-python:master.

@Nabellaleen
Copy link
Collaborator Author

I also fixed :

  • setup.py to really allow launching tests by using python setup.py test
  • README.md to give a working example for pytest arguments

@Nabellaleen Nabellaleen force-pushed the fix-enum-backport-requirement branch from 6a54188 to 1c2bdb9 Compare November 26, 2018 13:28
@Nabellaleen Nabellaleen requested a review from Cito March 18, 2019 10:00
@Nabellaleen Nabellaleen self-assigned this Mar 18, 2019
@Nabellaleen Nabellaleen added this to the 2.1.1 milestone Mar 18, 2019
Copy link
Member

@Cito Cito left a 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.

@Nabellaleen
Copy link
Collaborator Author

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

@jnak
Copy link
Collaborator

jnak commented Mar 27, 2019

@Nabellaleen I'm almost done with the migration to tox. I'll be sending a PR tomorrow because I'm off today.

@Nabellaleen
Copy link
Collaborator Author

@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 ;))

@jnak
Copy link
Collaborator

jnak commented Mar 27, 2019

@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",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
"rx<3.0.0",
"rx<3.0.0", # Rx 3.0 or above are only compatible with Python >= 3.6

@jnak
Copy link
Collaborator

jnak commented Mar 27, 2019

@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@a4d5700

@Nabellaleen
Copy link
Collaborator Author

Nabellaleen commented Mar 27, 2019

@jnak Yeah, I see ;) Great job ! I can't wait to see things move :D But I'll make an effort and be patient ;)

@Nabellaleen Nabellaleen force-pushed the fix-enum-backport-requirement branch from a0afe0e to 2db8fef Compare March 29, 2019 21:10
@Nabellaleen Nabellaleen merged commit 7e5c92d into graphql-python:master Mar 29, 2019
@Nabellaleen Nabellaleen deleted the fix-enum-backport-requirement branch March 29, 2019 21:27
@jnak
Copy link
Collaborator

jnak commented Mar 29, 2019

@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.

@Nabellaleen
Copy link
Collaborator Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants