-
Notifications
You must be signed in to change notification settings - Fork 137
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
fix(commands): Fix application command checks' typing #1048
Conversation
85f55fc
to
08c6e1b
Compare
No idea why RTD build fails, but other than that this PR is ready. |
Retriggering, might just be a quirk. |
d0b7082
to
a0b3886
Compare
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!
The addition of app_check(_any)
is a good idea, I don't see a way to make the existing decorator work with app commands either, especially in situations like @check(lambda inter: ...)
.
Co-authored-by: shiftinv <[email protected]> Signed-off-by: lena <[email protected]>
Done. |
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.
Sorry for the long wait, finally got around to looking at this again.
The check_any
decorator typing is still a bit iffy, but after spending some more time going down the rabbit hole of improving decorator typing more generally, it's out of scope for this PR.
…ix/check-typings
Co-authored-by: shiftinv <[email protected]> Signed-off-by: lena <[email protected]>
…isnake into fix/check-typings
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, thanks! Finally getting rid of all those # type: ignore
s in cogs is a great change.
Summary
This PR fixes typings related to application commands' checks (their addition/removal) and also implements
commands.app_check
andcommands.app_check_any
as an equivalent ofcommands.check
andcommands.check_any
for application commands as making existing function type hint with application commands is near to impossible.Fixes #1045
sorry for the .gitignore change
Checklist
pdm lint
pdm pyright