Closed pirogoeth closed 4 years ago
Looks mostly there. A few things...
Could you run the dev guide (in docs/dev/index.rst
) on your beanch to confirm they still work as written? Specifically:
Please run make style
to ensure style compliance. I really do need to add this step to the CI suite.
I think the make test
step will fail because at least one unit test expects that aiohttp
is installed. This could be resolved by changing the install instruction from pip install -e .
to pip install -e .[aiohttp]
or wrapping the unit tests that need aiohttp
with a @unittest.skipUnless(...)
check similar to that used in the tests/test_quart.py
file. I prefer the first option.
Don't worry about the make check_types
step failing - it does not work in master either. I have not been able to get that to work cleanly yet.
@claws Do you think it would be reasonable to have the pip install -r requirements.dev.txt
install the complete development environment by including -e .[aiohttp]
in the requirements.dev.txt
itself?
I've added a commit which introduces the above. If you don't like it, I've no qualms reverting and adjusting the documentation accordingly :)
I have push a new release of aioprometheus (19.11.0) to PyPI which contains your changes.
Includes:
aiohttp
extra for both theService
andPusher
featurespusher
andservice
Pusher
andService
initializers, to retain consistency in the top-level APIaiohttp.web.*
might not be available@claws let me know if there is anything I missed or if you have any concerns, thanks! :)