-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Improve set_webhook #2419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve set_webhook #2419
Conversation
I can, I did, I can confirm it worked. |
All reactions
Sorry, something went wrong.
On the last run, Codecov didn't show results for some reason, but the PR looks okay in the dashboard. |
All reactions
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
All reactions
jh0ker
Successfully merging this pull request may close these issues.
Check if we can be smarter about start_webhook
Note:
For people stumbling upon this: After releasing this PR in v13.4 we became aware that is was unintentioally breaking. See here for details.
When ready closes #2416
Already tested
cert
to TG) & with self-signed certificate (passingcert
to TG)To be done:
clean
to Deprecation: v15 #2347This also
clean
argument in favor ofdrop_pending_updates
as a) that conforms with the TG nomenclature, b) that name is more speakingip_address
parameter ofset_webhook
tostart_webhook
(tbh I don't really understand what it's good for, do we need some special casing for that?)When ready:
Checklist for PRs
.. versionadded:: version
,.. versionchanged:: version
or.. deprecated:: version
to the docstrings for user facing changes (I did that but leaving it as todo here so we double check the version number before merging)