-
-
Notifications
You must be signed in to change notification settings - Fork 636
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 debugpy's capabilities #15946
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
return ("-c", f"import {main.module};{main.module}.{main.function}();") | ||
return ("-m", main.module) | ||
|
||
assert isinstance(main, ConsoleScript) |
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.
It seems like this method should take a ConsoleScript
so that the caller is forced to apply (and reason about) this assertion instead.
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.
Well either entrypoint or console script, I'll change the type to a union
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.
Actually, I think I'm going to leave it as-is. These are the only two subclasses of MainSpecification
, and if I change it to use the union, the callsites needs to be casted or asserted, and neither is pretty.
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.
The assert is still happening: it's just hidden. IMO, that's a trap for the caller. But nbd.
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.
Yeah OK, thats fair. Past Josh was lazy, we should shun him.
- Adding an `args` flag for additional flags (especially useful for debug logs) - Adds support for executing module/module+function/console_script [ci skip-rust] [ci skip-build-wheels]
args
flag for additional flags (especially useful for debug logs)[ci skip-rust]
[ci skip-build-wheels]