Update setup configuration and create initial tests#45
Update setup configuration and create initial tests#45SmileyChris merged 11 commits intographql-python:masterfrom
Conversation
KingDarBoja
left a comment
There was a problem hiding this comment.
Overall looks good to me, just thinking about it, this package should implement black formatting, isort and check-manifest as part of the current tox step as other graphql-python repos does.
|
|
||
| from .base import BaseConnectionContext, BaseSubscriptionServer | ||
|
|
||
| __all__ = ['BaseConnectionContext', 'BaseSubscriptionServer'] |
There was a problem hiding this comment.
As far as I know, this line is supposed to export only the declared functions and "avoid" any other internal import from those modules.
There was a problem hiding this comment.
There's nothing that use these imports internally, the code all references the explicit base. These two classes aren't really usable externally, except perhaps as the base to your own third-party server. For a project that has languished so long, I don't see that being too much of a concern though - people can just update their code to import from the explicit base module too.
| [metadata] | ||
| name = graphql-ws | ||
| version = 0.3.1 | ||
| description = Websocket server for GraphQL subscriptions | ||
| long_description = file: README.rst, CHANGES.rst | ||
| author = Syrus Akbary | ||
| author_email = me@syrusakbary.com | ||
| url = https://github.com/graphql-python/graphql-ws | ||
| keywords = graphql, subscriptions, graphene, websockets | ||
| license = MIT | ||
| classifiers = | ||
| Development Status :: 2 - Pre-Alpha | ||
| Intended Audience :: Developers | ||
| License :: OSI Approved :: MIT License | ||
| Natural Language :: English | ||
| Programming Language :: Python :: 2 | ||
| Programming Language :: Python :: 2.6 | ||
| Programming Language :: Python :: 2.7 | ||
| Programming Language :: Python :: 3 | ||
| Programming Language :: Python :: 3.3 | ||
| Programming Language :: Python :: 3.4 | ||
| Programming Language :: Python :: 3.5 | ||
| Programming Language :: Python :: 3.6 | ||
| Programming Language :: Python :: 3.7 | ||
| Programming Language :: Python :: 3.8 |
There was a problem hiding this comment.
wasn't it better to keep this kind of information separate on the setup.py file and keep only formatting config on this file?
There was a problem hiding this comment.
eh, I dunno it's all bikeshed painting. I like having nothing in my setup.py personally (eventually we won't need it, hopefully :D)
|
Thanks for the review @KingDarBoja. I've got some more changes ready to deduplicate all the shared code and black the project, but wanted to do it in chunks rather than 1 giant PR |
This whole project has no tests. Here are some base graphql_ws tests, and basic tests for gevent, aiohttp and django_channels.
Rather than use an all-in-one
requirements_dev.txtfile, I've switched to using the optional install functionality of setuptools. Developers should install the project likepip install -e .[dev], maintainers (who also want to run tests) can dopip install -e .[dev,maintainer].