Add Defaults.run_async support#2210
Add Defaults.run_async support#2210Bibo-Joshi merged 9 commits intopython-telegram-bot:masterfrom starry-shivam:Defaults.run_async
Conversation
Signed-off-by: starry69 <starry369126@outlook.com>
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the quick PR!
Besides some nitpicks that I left, please make sure that pre-commit & codecov are happy. please see the contribution guide on how to run pre-commit locally. tests/test_dispatcher.py would be, where there new tests should go and you can take inspiration from the existing tests :)
we can ignore test_official for now, as we didn't merge API 5.0 yet …
Signed-off-by: starry69 <starry369126@outlook.com>
|
Will look into pre-commit and tests for |
Signed-off-by: starry69 <starry369126@outlook.com>
|
@Bibo-Joshi is everything alright now? |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
thanks for the update. apart from pre-commit not being happy, the test doesn't really make sense to me :/
Signed-off-by: starry69 <starry369126@outlook.com>
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Please also add a test for the error handler similar to the othe new test. in pycharm you can easily run the tests with coverage, btw, so you can check for coverage before pushing.
and please run pre-commit locally (see contribution guide) ;)
Signed-off-by: starry69 <starry369126@outlook.com>
|
Yeah sure, I'll add test for err handler aswell... And yeah i did run pre-commit locally but dunno why mypy is failing here, as it was passing in my local environment |
mypy isn't failing, black and flake8 are ;) |
Signed-off-by: starry69 <starry369126@outlook.com>
Umm those were passing aswell ^_^ |
Signed-off-by: starry69 <starry369126@outlook.com>
Bibo-Joshi
left a comment
There was a problem hiding this comment.
we're close :) left some nitpicks and also black still fails. you can run it manually with black . before commiting, don't know why pre-commit doesn't fail for you …
|
Thanks for reviewing! I'll address suggested changes in evening.. |
Signed-off-by: starry69 <starry369126@outlook.com>
|
@Bibo-Joshi is everything alright now? |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
just one documentation nitpick left :) btw, github already notifies me on updates ;)
telegram/ext/defaults.py
Outdated
| tzinfo (:obj:`tzinfo`): A timezone to be used for all date(time) objects appearing | ||
| throughout PTB. | ||
| run_async (:obj:`bool`): Optional. Default setting for the ``run_async`` parameter of | ||
| handlers and error handlers registered through :meth:`Dispatcher.add_handler`. |
There was a problem hiding this comment.
please link both add_handler and add_error_handler at the right places ;)
Signed-off-by: starry69 <starry369126@outlook.com>
|
Thanks for your contribution and your patience :) |
Adds
run_asyncsupport intelegram.ext.Defaultsas discussed in: #2208