-
Notifications
You must be signed in to change notification settings - Fork 145
refactor: Improve webhook typing. #1193
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
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for the PR! c:
It's not quite clear whether this is meant to be a typing or runtime change; followup is annotated as returning an InteractionFollowupWebhook, but returns a "plain" Webhook in practice.
Types that are only used in annotations can be confusing to some people, so it might be better to actually use this type at runtime too?
As a more general comment: having to subclass something to change a return type annotation for specific situations is rather awkward. I can't think of another solution either, other than maybe generics, which wouldn't help much iac.
Making it actually return an |
ah, gotcha. 🤔 |
That's a nice idea, thanks! |
|
Done in ec9a610! |
This is why those situations should be working on distinct types. Not a subclass of one handle-all type, as it later shows the interface allows for more than is actually supported. |
So you suggest something like making |
I suggest making |
Yeah. A bunch of the class/instance methods on |
Summary
Fixes #626
This is a typing-breaking change.
Should
Webhook.editandWebhook.deletealso be overloaded with aNeversomewhere to forbid calling them (you can't delete or edit an interaction webhook, right?)?Checklist
pdm lintpdm pyright