-
Notifications
You must be signed in to change notification settings - Fork 14
[FEATURE] Adds type validation GitHub action #96
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
[FEATURE] Adds type validation GitHub action #96
Conversation
f7c3f2b
to
a3938d1
Compare
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.
👏 praise: couple of comments but looking good 👍
0beebf2
to
2012163
Compare
c70a7e3
to
adc8c10
Compare
e984de2
to
990e7bc
Compare
learnosity_sdk/request/init.py
Outdated
self.secret = secret | ||
self.request = request | ||
if hasattr(request, 'copy'): | ||
# TO DO: Fix improper handling when request is a string (bug relevead by proper typing). Issue: AIL-415 |
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.
🤓 nitpick:
# TO DO: Fix improper handling when request is a string (bug relevead by proper typing). Issue: AIL-415 | |
# TODO: Fix improper handling when request is a string |
990e7bc
to
66bec37
Compare
66bec37
to
1af8eca
Compare
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.
Great work 🚀
We'll follow up on that typing issue that was caught but not introduced here as well 👍
Problem Statement
Since PR #81 is merged (relating to typing), a workflow (Github Action CI) should be added to ensure that typing is enforced, consistent and so that it is not out of sync over time.
PR Actions
Follow up work after PR #81.
What was done
--strict
setting).How was this change tested
mypy --strict learnosity_sdk
to verify all type hints and issues.Notes:
learnosity_sdk/request/init.py
was failing the MyPy check. The main changes to the logic were handling the cases where the optional request parameter was not provided (i.e. wasNone
). I chose to raise errors in the cases where request wasNone
(but needed to a dictionary) to avoid introducing logic and potentially silent errors / bugs.