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.

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

Expand type checking to the whole src folder #642

Open
tsusanka opened this issue Oct 22, 2019 · 6 comments
Open

Expand type checking to the whole src folder #642

tsusanka opened this issue Oct 22, 2019 · 6 comments
Assignees
Labels
code Code improvements core Trezor Core firmware. Runs on Trezor Model T and T2B1.

Comments

@tsusanka
Copy link
Contributor

Currently we run mypy on src/main.py, which means not everything in the src folder is being tested by mypy.

@tsusanka tsusanka added core Trezor Core firmware. Runs on Trezor Model T and T2B1. S4 Low code Code improvements labels Oct 22, 2019
@tsusanka tsusanka added this to the backlog milestone Oct 22, 2019
@ZdenekSL ZdenekSL added the W13 label Oct 24, 2019
@mmilata
Copy link
Member

mmilata commented Aug 21, 2020

Would be also nice to generate HTML report when running mypy: https://mypy.readthedocs.io/en/stable/command_line.html#report-generation

And perhaps fail CI if type coverage falls bellow threshold like we do for test coverage?

@tsusanka
Copy link
Contributor Author

I totally agree! Maybe we can add the CI check already? So we make sure the coverage does not degrade.

@mmilata
Copy link
Member

mmilata commented Aug 21, 2020

In the current state the report only shows main.py which is not very useful:) Passing all .py files to mypy makes it print a lot of warnings so we'd have to sort through all of them. I can look into it, maybe there's a way to generate reports for the set of files we currently check, then we can gradually expand this set ...

@tsusanka
Copy link
Contributor Author

tsusanka commented Aug 22, 2020

It was nonsense. I was thinking about adding a CI job to check if the current mypy coverage did not degrade but we obviously already have that - it is part of make style_check.

@tsusanka tsusanka removed W13 labels Feb 19, 2021
@tsusanka tsusanka removed this from the backlog milestone Oct 6, 2021
@tsusanka tsusanka added MEDIUM and removed LOW labels Oct 7, 2021
@grdddj grdddj linked a pull request Oct 27, 2021 that will close this issue
@grdddj grdddj linked a pull request Nov 1, 2021 that will close this issue
@matejcik
Copy link
Contributor

mostly fixed via #1939

what remains is the Monero app, for which I'm keeping this issue open

@hynek-jina hynek-jina added MEDIUM and removed HIGH labels Feb 24, 2022
@hynek-jina hynek-jina changed the title Expand mypy to the whole src folder Expand type checking to the whole src folder Feb 24, 2022
@hynek-jina
Copy link

Monero remains to be done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code improvements core Trezor Core firmware. Runs on Trezor Model T and T2B1.
Projects
Status: No status
7 participants