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

Rollout plan for mypy checks as part of general workflow #2461

Closed
dgw opened this issue Jun 4, 2023 · 3 comments · Fixed by #2502
Closed

Rollout plan for mypy checks as part of general workflow #2461

dgw opened this issue Jun 4, 2023 · 3 comments · Fixed by #2502
Milestone

Comments

@dgw
Copy link
Member

dgw commented Jun 4, 2023

Some mypy errors that made it into master, which I've been working on this week (patches that became #2462), served as a good demonstration to me that it's probably time to start running type checks as part of CI and make qa.

Assuming I haven't forgotten anything, we'll want a PR that rolls out these changes:

  • Add mypy target to make qa sequence
  • Update pull request template checklist to mention mypy in addition to quality and test targets
  • Add Actions step to run mypy at the correct point in CI (after flake8?)

Care will have to be taken with previously-approved PRs, but any that receive changes after merging the updated CI workflow will have any new errors flagged.


Future idea: Add a Makefile target to run the full GHA suite, using something like nektos/act
Not opening its own issue just yet because it's a whole kettle of fish to set up act, which itself requires Docker, which has its own system requirements—then select the correct images to actually behave like a GHA runner, and those images are not small.

@dgw dgw added Tests Build Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. labels Jun 4, 2023
@dgw
Copy link
Member Author

dgw commented Jun 11, 2023

I've started working on the errors left behind now that #2462 is merged, by checking the output of mypy --check-untyped-defs sopel. That's in a mypy-untyped-defs branch, and I think if we're going to start enforcement we might could wait until the rest of the glaring issues are solved. (But if someone else wants to press ahead with CI enforcement sans flag, sure, go ahead.)

@dgw
Copy link
Member Author

dgw commented Jun 11, 2023

Oh, another reason to delay enforcement a little longer is that there's at least one error in sopel/__init__.py that requires a fix to mypy that hasn't been included in a release yet. It was merged a week before mypy 1.3, but wasn't included in that tag; it's been pulled into the upstream release-1.4 branch, though.

@dgw dgw removed the Long-term Planning Things that need to happen at some point in the future, but need to NOT happen soon. label Aug 16, 2023
@dgw dgw added this to the 8.0.0 milestone Aug 16, 2023
@dgw
Copy link
Member Author

dgw commented Aug 16, 2023

mypy is up to v1.5.0 on PyPI now, so the fix we were waiting for should be present going forward. And we already have verification that CI passes with mypy type-checking as part of the process thanks to #2502.

@dgw dgw closed this as completed in #2502 Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant