-
Notifications
You must be signed in to change notification settings - Fork 88
type Command.force
#382
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
type Command.force
#382
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,11 +109,11 @@ class build_ext(Command): | |
| ('help-compiler', None, "list available compilers", show_compilers), | ||
| ] | ||
|
|
||
| def initialize_options(self): | ||
| def initialize_options(self) -> None: | ||
| self.extensions = None | ||
| self.build_lib = None | ||
| self.plat_name = None | ||
| self.build_temp = None | ||
| self.build_lib: str = None # Should always be set in finalize_options | ||
| self.plat_name: str = None # Should always be set in finalize_options | ||
| self.build_temp: str = None # Should always be set in finalize_options | ||
|
Comment on lines
+114
to
+116
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Avasam This change was necessary to get the build to pass. Is this also acceptable?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not quite:
If you HAVE to decide, I'd say keeping the type accurate to AFTER running But all is not lost, there's one more trick you can use we developed in typeshed:
(it'll obviously reduce type safety, but false-negatives are generally preferred over false-positives, especially for a legacy system like this)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case of doubt, let's keep working through other PRs and I can tackle this more in depth
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I follow, but this does allow the tests to pass, so let's go with it for now and we can refine subsequently. Thanks.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the new failing test results in #368 for what I meant |
||
| self.inplace = False | ||
| self.package = None | ||
|
|
||
|
|
@@ -125,7 +125,7 @@ def initialize_options(self): | |
| self.rpath = None | ||
| self.link_objects = None | ||
| self.debug = None | ||
| self.force = None | ||
| self.force: bool = None # Should always be set in finalize_options | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not actually type-safe and easy to break in the future, but this is only place in distutils we assume a type for after Alternatively this can be kept as |
||
| self.compiler = None | ||
| self.swig = None | ||
| self.swig_cpp = None | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.