Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Refactor HomeserverConfig so it can be typechecked#6137

Merged
hawkowl merged 34 commits intodevelopfrom
hawkowl/config-cleanup
Oct 10, 2019
Merged

Refactor HomeserverConfig so it can be typechecked#6137
hawkowl merged 34 commits intodevelopfrom
hawkowl/config-cleanup

Conversation

@hawkowl
Copy link
Copy Markdown
Contributor

@hawkowl hawkowl commented Sep 30, 2019

This means that static typing can now functionally be used, if we define type stubs/mypy can infer things

@hawkowl hawkowl requested a review from a team October 3, 2019 13:54
@hawkowl hawkowl marked this pull request as ready for review October 3, 2019 13:54
Copy link
Copy Markdown
Member

@richvdh richvdh left a 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

Comment thread mypy.ini
[mypy-signedjson.*]
ignore_missing_imports = True

[mypy-prometheus_client.*]
Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Member

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?

Comment thread synapse/config/_base.py Outdated
Comment thread synapse/config/_base.py Outdated
Comment thread synapse/config/_base.py Outdated
Comment thread synapse/config/_base.py
Comment thread synapse/config/_base.py Outdated
Comment thread tox.ini
@hawkowl hawkowl requested review from a team and richvdh and removed request for richvdh October 8, 2019 13:43
Copy link
Copy Markdown
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@hawkowl hawkowl merged commit f743108 into develop Oct 10, 2019
@hawkowl hawkowl deleted the hawkowl/config-cleanup branch October 10, 2019 08:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants