Member
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. see comments if you care to change anything.
Comment on lines +96 to +97
| * For Socks5 support, additional dependencies are required. Make sure to install | ||
| PTB via :command:`pip install "python-telegram-bot[socks]"` in this case. |
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
| * For Socks5 support, additional dependencies are required. Make sure to install | |
| PTB via :command:`pip install "python-telegram-bot[socks]"` in this case. | |
| * For Socks5 support, additional dependencies are required. They can be installed using the | |
| command :command:`pip install "python-telegram-bot[socks]"`. |
# Conflicts: # telegram/request/_httpxrequest.py
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ApplicationBuilder.proxy_url part should be changed as well? Shouldn't the tests fail because of the uncaught warning raised?
Member Author
The
ApplicationBuilder.proxy_urlpart should be changed as well? Shouldn't the tests fail because of the uncaught warning raised?
Sorry, hadn't seen this. Will change appbilder. PTB warnings are explicitly ignored in conftest.py so that we don't have to catch them on every test
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look good, just some typos:
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm