May 18 minor changes - #628
Conversation
Keep old for now... we can remove it in the future when telegram stops parsing it along. Also: TODO: write proper Message tests
tsnoam
left a comment
tsnoam
left a comment
There was a problem hiding this comment.
See comments on code.
Other than that, looks good :)
| thumb_url, | ||
| mpeg4_width=None, | ||
| mpeg4_height=None, | ||
| mpeg4_duration=None, |
There was a problem hiding this comment.
We had discussions about this in the past. Should new parameters be added in the location where Telegram documentation added them or in the end of the parameters list (before kwargs) to avoid backward compatibility issues.
I prefer the latter.
@jh0ker what do you think?
| @@ -168,6 +169,7 @@ def __init__(self, | |||
| self.location = location | |||
| self.venue = venue | |||
| self.new_chat_member = new_chat_member | |||
There was a problem hiding this comment.
I think we should help our users by storing the value on self._new_chat_member and make new_chat_member a property + deprecation warning.
| return self.bot.getUserProfilePhotos(self.id, *args, **kwargs) | ||
|
|
||
| @staticmethod | ||
| def de_list(data, bot): |
There was a problem hiding this comment.
Just saying (don't think anything should be changed atm):
JSON can also be a list, so our notion of de_json & to_json should be fixed. (future todo?)
There was a problem hiding this comment.
Hmm, so you're saying we shouldn't both have a de_list and a de_json, but that de_json should do both?
There was a problem hiding this comment.
yes. but not now. we need to open an issue so we won't forgot.
| thumb_url, | ||
| gif_width=None, | ||
| gif_height=None, | ||
| gif_duration=None, |
There was a problem hiding this comment.
Same comment like mpeg4_duration below.
Actually shows where in the users code the error happened, not just where the warning came from in our internal code
2 participants
refs #617