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

Update mypy and run it on all the files #37

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

Swatinem
Copy link
Contributor

Running mypy only on the changed files is the wrong thing to do, as typing changes in one file can have an effect on upstream code that imports said file.

Apart from removing that file filtering, this also updates the setup-python GHA, and mypy itself.


TBH, I don’t think this should be a centralized workflow at all. mypy should really be installed as a dev dependency per-repo, and run within that repo, using the local .python-version as well, so the python and mypy versions are defined within that repo.
As both python updates and mypy updates could have an effect on the results of type checking.

Running `mypy` only on the changed files is the wrong thing to do, as typing changes in one file can have an effect on upstream code that imports said file.

Apart from removing that file filtering, this also updates the `setup-python` GHA, and `mypy` itself.
@Swatinem Swatinem requested a review from a team November 18, 2024 10:18
@Swatinem Swatinem self-assigned this Nov 18, 2024
Copy link
Contributor

@giovanni-guidini giovanni-guidini left a comment

Choose a reason for hiding this comment

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

I agree that mypy should be a dev-dependency of projects and use the local python of the project

@Swatinem Swatinem merged commit 92eeeab into main Nov 18, 2024
1 check passed
@Swatinem Swatinem deleted the swatinem/mypy-all-the-things branch November 18, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants