Skip to content

Conversation

@Poolitzer
Copy link
Member

@Poolitzer Poolitzer commented Feb 16, 2024

This is based against the deprecations so we have less merge pain

Checklist

  • Added .. versionadded:: NEXT.VERSION, .. versionchanged:: NEXT.VERSION or .. deprecated:: NEXT.VERSION to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Documented code changes according to the CSI standard <https://standards.mousepawmedia.com/en/stable/csi.html>__
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs and all suitable __all__ s
  • Checked the Stability Policy <https://docs.python-telegram-bot.org/stability_policy.html>_ in case of deprecations or changes to documented behavior

If the PR contains API changes (otherwise, you can ignore this passage)

  • Checked the Bot API specific sections of the Stability Policy

  • New classes:

    • Added self._id_attrs and corresponding documentation
    • __init__ accepts api_kwargs as kw-only
  • Added new shortcuts:

    • In :class:~telegram.Chat & :class:~telegram.User for all methods that accept chat/user_id
    • In :class:~telegram.Message for all methods that accept chat_id and message_id
    • For new :class:~telegram.Message shortcuts: Added do_quote argument if methods accepts reply_to_message_id
    • In :class:~telegram.CallbackQuery for all methods that accept either chat_id and message_id or inline_message_id
  • If relevant:

    • Added new constants at :mod:telegram.constants and shortcuts to them as class variables
    • Link new and existing constants in docstrings instead of hard-coded numbers and strings
    • Add new message types to :attr:telegram.Message.effective_attachment
    • Added new handlers for new update types
    • Add the handlers to the warning loop in the :class:~telegram.ext.ConversationHandler
    • Added new filters for new message (sub)types
    • Added or updated documentation for the changed class(es) and/or method(s)
    • Added the new method(s) to _extbot.py
    • Added or updated bot_methods.rst
    • Updated the Bot API version number in all places: README.rst and README_RAW.rst (including the badge), as well as telegram.constants.BOT_API_VERSION_INFO
    • Added logic for arbitrary callback data in :class:telegram.ext.ExtBot for new methods that either accept a reply_markup in some form or have a return type that is/contains :class:~telegram.Message

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Looks like you edited README.rst or README_RAW.rst. I'm just a friendly reminder to apply relevant changes to both of those files :)

@Poolitzer Poolitzer added the ⚙️ bot-api affected functionality: bot-api label Feb 16, 2024
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial review

Filters and Message.effective_attachment needs to be updated

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fast PR!

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I noticed the workflows are running twice so that needs to be fixed also

test_official should also be updated to ignore the 3 new optional param -> required param

@Bibo-Joshi
Copy link
Member

also I noticed the workflows are running twice so that needs to be fixed also

This is an important observation. Looking back at the workflow changes, I think we don't need to run many workflows on pushes to branches. master is the only branch where this really makes sense and I would like to see all tests running on master. Running the doc test on push to doc-fixes is okay for me, but not strictly necessary.
For everything else, we'll have PRs anyways, so it suffices to see the tests on on them.

I noticed that the doc-test worflow did not run on this PR. this is probably due to:

If you define both branches/branches-ignore and paths/paths-ignore, the workflow will only run when both filters are satisfied.

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onpull_requestpull_request_targetbranchesbranches-ignore

So in docs.yml the branches condition should be removed for both push and pull_request.

@Poolitzer
Copy link
Member Author

I added basic bool filter. For all of them more arguments could be supported, but I thought we could wait for this if people actually want that.

Base automatically changed from remove-api-7.0-deprecations to master February 25, 2024 09:34
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a 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! Just a bit more nitpicking.
test_official needs to be adjusted to deal with the temporariy deviation of the can_* being required. IISC the following needs to be updated for that:

IGNORED_PARAM_REQUIREMENTS = {
# Ignore these since there's convenience params in them (eg. Venue)
# <----
"send_location": {"latitude", "longitude"},
"edit_message_live_location": {"latitude", "longitude"},
"send_venue": {"latitude", "longitude", "title", "address"},
"send_contact": {"phone_number", "first_name"},
# ---->
}

Please leave a comment including NEXT.VERSION and "API 7.1" so we have a chance of finding that again when actually making these arguments required :)

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good to me. We can incorporate https://t.me/bot_api_changes/123 into doc-fixes later.

@Bibo-Joshi Bibo-Joshi merged commit 099ab5d into master Mar 2, 2024
@Bibo-Joshi Bibo-Joshi deleted the api-7.1 branch March 2, 2024 09:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⚙️ bot-api affected functionality: bot-api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants