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

Support v3.6 API #1006

Merged
merged 10 commits into from Feb 18, 2018

Conversation

Projects
None yet
6 participants
@code1mountain
Contributor

code1mountain commented Feb 14, 2018

Added support for connected_website from Bot API v3.6.

Update: Also added support for parse_modes in captions. I added only tests for parse_mode in captions in tests/test_photo.py, because adding a test for all kind of messages that contain a caption is redundant.

Update 2: I added the support for the parse_mode parameter in all InlineQueryResults

Update 3: Added the support streaming parameter to telegram.Bot.send_video and InputMediaVideo. The streaming parameter will not work for any videos you send by file_id, that don't support streaming already or sending by file or url for files, the telegram server already downloaded once. Anyways, Telegram won't raise an Error for that.

Fixes part 1 + 3 + 4 from issue #1005

@code1mountain code1mountain changed the title from Support for connected_website to Support for connected_website + parse_mode in captions Feb 14, 2018

@Eldinnie Eldinnie changed the title from Support for connected_website + parse_mode in captions to Support v3.6 API Feb 14, 2018

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Feb 14, 2018

Member

For Now I renamed the PR, since it already contains two of the newly added features. Will look in depth later.

Member

Eldinnie commented Feb 14, 2018

For Now I renamed the PR, since it already contains two of the newly added features. Will look in depth later.

@jeffffc

This comment has been minimized.

Show comment
Hide comment
@jeffffc

jeffffc Feb 14, 2018

Contributor

I think the InlineQueryResult* objects contains new parse_mode field too

Contributor

jeffffc commented Feb 14, 2018

I think the InlineQueryResult* objects contains new parse_mode field too

@code1mountain

This comment has been minimized.

Show comment
Hide comment
@code1mountain

code1mountain Feb 14, 2018

Contributor

Yeah, I just saw that parse_mode also affects InlineQueryResult*. I will add it asap

Contributor

code1mountain commented Feb 14, 2018

Yeah, I just saw that parse_mode also affects InlineQueryResult*. I will add it asap

mathefreak1 added some commits Feb 14, 2018

Added supports_streaming parameter in telegram.Bot.send_video and tel…
…egram.InputMediaVideo

Fixed Docstrings for parse_mode in captions

@Eldinnie Eldinnie changed the base branch from master to bot-api-3.6 Feb 15, 2018

@Eldinnie Eldinnie changed the base branch from bot-api-3.6 to master Feb 15, 2018

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Feb 15, 2018

Member

Ok, I finished up and made some adjustments.
Let's see what CI thinks

Member

Eldinnie commented Feb 15, 2018

Ok, I finished up and made some adjustments.
Let's see what CI thinks

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Feb 15, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@9338dc4). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff            @@
##             master   #1006   +/-   ##
========================================
  Coverage          ?   91.8%           
========================================
  Files             ?     103           
  Lines             ?    4136           
  Branches          ?     669           
========================================
  Hits              ?    3797           
  Misses            ?     197           
  Partials          ?     142
Impacted Files Coverage Δ
telegram/message.py 96.44% <100%> (ø)
telegram/inline/inlinequeryresultmpeg4gif.py 100% <100%> (ø)
telegram/inline/inlinequeryresultaudio.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedmpeg4gif.py 100% <100%> (ø)
telegram/inline/inlinequeryresultgif.py 100% <100%> (ø)
telegram/bot.py 87.78% <100%> (ø)
telegram/inline/inlinequeryresultvoice.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedphoto.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedvoice.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedgif.py 100% <100%> (ø)
... and 9 more

codecov bot commented Feb 15, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@9338dc4). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff            @@
##             master   #1006   +/-   ##
========================================
  Coverage          ?   91.8%           
========================================
  Files             ?     103           
  Lines             ?    4136           
  Branches          ?     669           
========================================
  Hits              ?    3797           
  Misses            ?     197           
  Partials          ?     142
Impacted Files Coverage Δ
telegram/message.py 96.44% <100%> (ø)
telegram/inline/inlinequeryresultmpeg4gif.py 100% <100%> (ø)
telegram/inline/inlinequeryresultaudio.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedmpeg4gif.py 100% <100%> (ø)
telegram/inline/inlinequeryresultgif.py 100% <100%> (ø)
telegram/bot.py 87.78% <100%> (ø)
telegram/inline/inlinequeryresultvoice.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedphoto.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedvoice.py 100% <100%> (ø)
telegram/inline/inlinequeryresultcachedgif.py 100% <100%> (ø)
... and 9 more

@Eldinnie Eldinnie referenced this pull request Feb 15, 2018

Closed

Api 3.6 #1008

Eldinnie added some commits Feb 16, 2018

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Feb 16, 2018

Member

Ok, diff coverage is up to 100%.
Review please, @tsnoam @jh0ker @bomjacob ?

Member

Eldinnie commented Feb 16, 2018

Ok, diff coverage is up to 100%.
Review please, @tsnoam @jh0ker @bomjacob ?

@jsmnbom

LGTM, good work on this @code1mountain and @Eldinnie.
After a fairly quick lookover I couldn't find any errors or anything like that :)

@tsnoam

@Eldinnie @code1mountain

In general, LGTM.
However, there are three places where the new function arguments needs to be moved to the end of the function prototype in order to maintain backward compatibility:

  • send_photo
  • send_audio
  • edit_message_caption

changing that and we're good to go

@tsnoam tsnoam referenced this pull request Feb 18, 2018

Merged

More instance methods #963

@tsnoam

This comment has been minimized.

Show comment
Hide comment
@tsnoam

tsnoam Feb 18, 2018

Member

Also needs to be fixed on send_voice

Member

tsnoam commented Feb 18, 2018

Also needs to be fixed on send_voice

@tsnoam

This comment has been minimized.

Show comment
Hide comment
@tsnoam

tsnoam Feb 18, 2018

Member

@code1mountain
I've created the needed changes but i can't seem to push into your repo. Have you enabled edits by maintainers?
https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/

Member

tsnoam commented Feb 18, 2018

@code1mountain
I've created the needed changes but i can't seem to push into your repo. Have you enabled edits by maintainers?
https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Feb 18, 2018

Member

@tsnoam Yes, because I did too

Member

Eldinnie commented Feb 18, 2018

@tsnoam Yes, because I did too

@tsnoam

This comment has been minimized.

Show comment
Hide comment
@tsnoam

tsnoam Feb 18, 2018

Member

my bad, i tried to push to the wrong branch.
now it's pushed, i'm waiting for travis and if it goes well i'll squash-merge.

Member

tsnoam commented Feb 18, 2018

my bad, i tried to push to the wrong branch.
now it's pushed, i'm waiting for travis and if it goes well i'll squash-merge.

@tsnoam

tsnoam approved these changes Feb 18, 2018

@tsnoam tsnoam merged commit c152d65 into python-telegram-bot:master Feb 18, 2018

5 checks passed

codeclimate All good!
Details
codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
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.

Show comment
Hide comment
@tsnoam

tsnoam Feb 18, 2018

Member

@code1mountain thanks for your contribution!

Member

tsnoam commented Feb 18, 2018

@code1mountain thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment