-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding MyPy to CI #90
Conversation
], | ||
[], | ||
) | ||
invalid_local_phone_numbers = [ |
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.
I executed this statement and found that it was just a really over-complicated way of building a list like this:
[("1234", "Not a valid local number"), ("2345", "Not a valid local number"), ...]
So I replaced it, verified that the new statement ==
the old, and the type error went away.
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.
Thanks for coming back on this; the code is much clearer to me with this new version.
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.
LGTM
Summary | Résumé
This PR adds the MyPy static type checker to notification-utils, similar to what we have done for notification-admin (cds-snc/notification-admin#1055) and notification-api (cds-snc/notification-api#1320).