-
Notifications
You must be signed in to change notification settings - Fork 92
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
Implement mypy annotations (Resolves #549) #557
Conversation
Squashed commit: see https://github.com/64bitpandas/ocfweb for full commit history
Ah, yeah we need to update the version of the django plugin to work with the new semantic analyzer. If you could update that and mypy itself, that would be great! I'll start reviewing this a bit later. |
Alright sounds good! I've fixed the first 2 notes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is excellent! Thank you so much for working on it. I have a few nits/suggestions here and there, but otherwise I look forward to this being merged :P
Thank you so much for putting in the time on this! It is greatly appreciated. |
This should make improper type usages much easier to catch and encourage better self-documentation in the future :)
Some notes:
Typing annotations are very much WIP and I usedAny
too liberally, especially forrequest: Any
since I was unsure what typerequest
is.request: Any
has been fixed and replaced withrequest: HttpRequest
Some of the
Any
keywords are used purposefully, but many can definitely be improved.Fixednew_semantic_analyzer
hasn't been enabled yet- an internal error occurs (see bottom of comment for stacktrace) and I figured that fixing it would be a whole other unrelated problem.