-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Improve pywin32.isapi
#13889
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 pywin32.isapi
#13889
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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! That's a nice improvement. I have a few small comments.
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! That's a nice improvement. I have a few small comments.
Co-authored-by: Avasam <samuel.06@hotmail.com>
def InstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, str], Unused] = ...) -> None: ... | ||
def UninstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, str], Unused] = ...) -> None: ... |
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.
should it be sync with the log
function?
def InstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, str], Unused] = ...) -> None: ... | |
def UninstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, str], Unused] = ...) -> None: ... | |
def InstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, object], Unused] = ...) -> None: ... | |
def UninstallModule(conf_module_name: StrPath, params, options, log: Callable[[int, object], Unused] = ...) -> None: ... |
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.
Good catch. Yeah.
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.
@donBarbos My bad. I answered too fast. Here it means "this function expects to be passed a log
parameter that should be a function that accepts a str
as the 2nd parameter".
In other words, we want to ensure that whatever function will be passed here will work once a string is passed. The typing here is based on usage, it's completely separate from isapi.isntall.log
I went ahead and reverted the last commit.
thanks for helping get rid of |
This comment has been minimized.
This comment has been minimized.
This reverts commit 5e6a2d7.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
No description provided.