Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Setup mypy in CI, and get full coverage of the sydent package #414

Closed
DMRobertson opened this issue Oct 8, 2021 · 3 comments
Closed

Setup mypy in CI, and get full coverage of the sydent package #414

DMRobertson opened this issue Oct 8, 2021 · 3 comments

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Oct 8, 2021 •
edited
Loading

This could have helped us to spot the problem fixed in #415 .

Edit: Related PRs:
@DMRobertson DMRobertson self-assigned this Oct 8, 2021
@DMRobertson
Copy link
Contributor Author

Progress so far: #416 #417 #418 #421

@DMRobertson DMRobertson mentioned this issue Oct 15, 2021
Merged
DMRobertson pushed a commit that referenced this issue Oct 15, 2021
Make sydent.threepid pass mypy --strict
c9d7ea7 
Part of #414
@DMRobertson DMRobertson mentioned this issue Oct 15, 2021
Merged
DMRobertson pushed a commit that referenced this issue Oct 15, 2021
Make sydent.terms pass mypy --strict
c2dca4d 
Shout out to getForClient, which a) mixes presentation with data and b)
was a massive PITA to type hint. It's very very stringly typed.

Part of #414
@DMRobertson DMRobertson mentioned this issue Oct 15, 2021
Merged
DMRobertson pushed a commit that referenced this issue Oct 15, 2021
Make sydent.sms pass mypy --strict
0636789 
I went a little mad with power adding TypeDicts to effictively provide a
schema for the OpenMarket API. It was an useful exercise though.

Part of #414.
This was referenced Oct 15, 2021
Merged
Merged
Merged
@DMRobertson DMRobertson changed the title Setup mypy in CI Setup mypy in CI, and get full coverage of the sydent package Oct 21, 2021
@DMRobertson
Copy link
Contributor Author

Will write down some more coherent thoughts in a minute but, here is a graph that demonstrates the change in MyPy's coverage:

image

The first data point comes from just before @H-Shay's work to add type hints over the summer. The grey stripe represents the PRs in this issue. For completeness, I'll stick up a branch with the script I used to harvest the data and generate the graph.

@DMRobertson
Copy link
Contributor Author

DMRobertson commented Oct 29, 2021 •
edited
Loading

That branch is https://github.com/matrix-org/sydent/tree/dmr/mypy-precision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@DMRobertson @H-Shay