-
Notifications
You must be signed in to change notification settings - Fork 6k
Cross references #3135
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
Cross references #3135
Conversation
harshil21
left a comment
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.
Nice work. Left some comments below. If it's not too much work, can you rebase your branch on the top of the doc-fixes branch? If done correctly, Github will only show your commits and remove the pool size commit.
Umm, harshil I dont know how to rebase branches 🤔 could you help me please? |
|
this article should get you started :) if you need more help, please ping us in the dev-chat on tg |
|
Why the codacy test is failing? |
|
It cant find the git difference for some reason |
harshil21
left a comment
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.
nice job rebasing, most of my comments is just now moving stuff above the Args section.
Bibo-Joshi
left a comment
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.
Hi. Sorry, I'm a bit late to the review-party … The additions are all nice 👍 I have three more general comments:
-
CallbackQueryhas a bunch of shortcut methods as well, likeedit_message_text. Can you check the documentation of that class and also link them in theBotclass? -
the
Userclass also has all thesend_*shortcuts, likeChatdoes. please also link these :) -
many
.. seealso::directives are in the wrong position in the docstring. The docstring should have the following order:- General description of the class/method
seealso(sometimes this can also be part of 1.).. versionadded::/.. versionchanged::- Arguments
- Attributes/Returns
Please move your additions accordingly
I also saw a few added newlines, that we don't really need, but that's not overly important.
Bibo-Joshi
left a comment
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.
Thanks for the updates! I left a few nitpick-comments :) In additon, the shortcut methods {User, Chat}.get/set_menu_button can also be linked in Bot.{get, set}_chat_menu_button
| by Telegram. | ||
| .. seealso:: :attr:`telegram.Message.reply_animation`, :attr:`telegram.Chat.send_animation` | ||
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.
User.send_animation
| .. seealso:: :attr:`telegram.Message.copy`, :attr:`telegram.Chat.send_copy`, | ||
| :attr:`telegram.Chat.copy_message`, :attr:`telegram.User.send_copy` | ||
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.
User.copy_message
telegram/ext/_contexttypes.py
Outdated
| .. seealso:: `contexttypesbot.py <https://github.com/python-telegram-bot/ | ||
| python-telegram-bot/blob/master/examples/contexttypesbot.py>`_ | ||
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.
| .. seealso:: `contexttypesbot.py <https://github.com/python-telegram-bot/ | |
| python-telegram-bot/blob/master/examples/contexttypesbot.py>`_ | |
| .. seealso:: `contexttypesbot.py <examples.contexttypesbot.html>`_ | |
the other links use this - I take it this was tested?
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.
yes
telegram/ext/_extbot.py
Outdated
| .. seealso:: `arbitrarycallbackbot.py <https://github.com/python-telegram-bot/ | ||
| python-telegram-bot/blob/master/examples/arbitrarycallbackdatabot.py>`_, |
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.
see above
telegram/ext/_inlinequeryhandler.py
Outdated
| chats and may not be set for inline queries coming from third-party clients. These | ||
| updates won't be handled, if :attr:`chat_types` is passed. | ||
| .. seealso:: `inlinebot.py <https://github.com/python-telegram-bot/ |
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.
s.a.
telegram/ext/_pollanswerhandler.py
Outdated
| When setting :paramref:`block` to :obj:`False`, you cannot rely on adding custom | ||
| attributes to :class:`telegram.ext.CallbackContext`. See its docs for more info. | ||
| .. seealso:: `pollbot.py <https://github.com/python-telegram-bot/ |
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.
.
telegram/ext/_pollhandler.py
Outdated
| When setting :paramref:`block` to :obj:`False`, you cannot rely on adding custom | ||
| attributes to :class:`telegram.ext.CallbackContext`. See its docs for more info. | ||
| .. seealso:: `pollbot.py <https://github.com/python-telegram-bot/ |
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.
.
|
Thank you very much for your contribution and the patience during the review process :) |
|
addresses #3110 (just adding for this PR to show up on the page of the issue) |
Supersedes #3134