-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add strictly typed responses; fix circular dependencies; improve namespaces #28
Conversation
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.
🥈 questions 🌚
:rtype: None | ||
""" | ||
|
||
import datetime |
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.
Why are we importing inside a function ?
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.
It is an exception made for the initializer to make sure no code from within the module is executed before it is needed. Reducing chances of circular deps!
@@ -29,25 +29,25 @@ class InstallationAdapter(converter.JsonAdapter): | |||
@classmethod | |||
def deserialize(cls, target_class, array): | |||
""" | |||
:type target_class: model.Installation|type | |||
:type target_class: core.Installation|type |
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.
Is this notation used every where else in this SDK as well ? 🙊
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.
Notation, as in |type
? Yes, absolutely! For long time already :)
Same goes for module.Class
! 👍
@OGKevin all questions answered |
No description provided.