-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Refactor HomeserverConfig so it can be typechecked #6137
Conversation
…synapse into hawkowl/config-cleanup
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.
looks good, but a few questions
@@ -52,3 +48,15 @@ ignore_missing_imports = True | |||
|
|||
[mypy-signedjson.*] | |||
ignore_missing_imports = True | |||
|
|||
[mypy-prometheus_client.*] |
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.
why are we adding this sort of thing?
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.
because we don't have type stubs for these modules, so therefore we don't want to typecheck them. We don't want to ignore missing imports completely, just on dependencies without type hints.
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.
ok, but it just doesn't seem related to the subject of the PR and the text of the changelog.
I guess it's necesary because of https://github.com/matrix-org/synapse/pull/6137/files#diff-b91f3d5bd63fcd17221b267e851608e8R169?
Co-Authored-By: Richard van der Hoff <[email protected]>
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.
This means that static typing can now functionally be used, if we define type stubs/mypy can infer things