Update Tox, Add MyPy, .pre-commit configurations #18
Conversation
…t - flake8 currently fails on files, to be fixed in other commit
| "pytest-benchmark", | ||
| "pytest-cov", | ||
| "pytest-mock", | ||
| "snapshottest", |
There was a problem hiding this comment.
are all of these really required to run tests? In the before diff there was only pytest.
There was a problem hiding this comment.
Probably not all of them, but we weren't running coverage before. (coverage is at 66pct with the current config, which seems low for a graphql python lib)
| deps = | ||
| pre-commit>0.12.0 | ||
| setenv = | ||
| LC_CTYPE=en_US.UTF-8 |
There was a problem hiding this comment.
what are these 3 environment vars for?
There was a problem hiding this comment.
So graphene has the locale set for en US I, I believe, however when running on a system that handles the others (which are preferred) black has issues. I'll dig out the error I had tomorrow that made these go away
|
|
||
| [testenv:flake8] | ||
| basepython=python3.6 | ||
| deps = flake8 | ||
| commands = | ||
| pip install -e . |
There was a problem hiding this comment.
I don't usually see pip install in a tox.ini testenv since tox takes care of creating virtualenv for you with the deps specified. I'm not that familiar with this so feel free to ignore this comment, but is there a more tox-y way to do this?
There was a problem hiding this comment.
If so, can we make the same change on graphene? I was referring to that as the source of truth on how to do it :)
https://github.com/graphql-python/graphene/blob/master/tox.ini
Cito
force-pushed
the
master
branch
2 times, most recently
from
d21bdd4 to
c781f01
Compare
|
Superseded by #35 |
99% consistent with graphene