Skip to content
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 instance methods to Animation and ChatPhoto #1489

Merged
merged 7 commits into from Sep 13, 2019

Conversation

@zeshuaro
Copy link
Contributor

zeshuaro commented Aug 28, 2019

No description provided.

Copy link
Member

Bibo-Joshi left a comment

Thanks for this PR, @zeshuaro! I think this is a good addition. A few notes:

  • Please add telegram.Animation to the doc string of bot.get_file
  • The updated tests seem to be copy-pasted from test_audio, so they should be fine. I'd still like to have one @python-telegram-bot/maintainers review them …
  • While you're at it, do you thinks you could add get_small_file and get_big_file to ChatPhoto ?

AFAICS the failed tests are only due to test_official, but the test setup was updated to allow for that to fail. So if merge from master, we can see, if everthing passes :)

tests/test_animation.py Outdated Show resolved Hide resolved
@zeshuaro zeshuaro changed the title Add get_file to Animation Add instance methods to Animation and ChatPhoto Sep 6, 2019
zeshuaro added 2 commits Sep 6, 2019
@zeshuaro

This comment has been minimized.

Copy link
Contributor Author

zeshuaro commented Sep 9, 2019

I've fixed the issues that you pointed out and added the new methods.

@Bibo-Joshi

This comment has been minimized.

Copy link
Member

Bibo-Joshi commented Sep 9, 2019

Yes, thanks!
The failed test seems unrelated, so this get's a LGTM from my side. :)

@tsnoam tsnoam merged commit 32dd415 into python-telegram-bot:master Sep 13, 2019
4 of 5 checks passed
4 of 5 checks passed
codecov/patch 60% of diff hit (target 92.86%)
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
codecov/project 92.97% (+0.11%) compared to d5399de
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tsnoam

This comment has been minimized.

Copy link
Member

tsnoam commented Sep 13, 2019

@zeshuaro Thank you for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.