Skip to content

Conversation

Poolitzer
Copy link
Member

Checklist for PRs

  • Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: 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
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs and all suitable __all__ s

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

  • If relevant:

    • Added new constants at telegram.constants and shortcuts to them as class variables
    • Link new and existing constants in docstrings instead of hard coded number and strings
    • Added new filters for new message (sub)types
    • Added or updated documentation for the changed class(es) and/or method(s)
    • 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

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 :)

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.

Neat! :) Had a first look through the changes and left some smaller comments below. Will try to have a look at the tests later

@harshil21 harshil21 added this to the v20.0a2 milestone Jun 21, 2022
@harshil21 harshil21 added the ⚙️ bot-api affected functionality: bot-api label Jun 21, 2022
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.

Fast job!

To change:

  • parameter login_url of LoginUrl should be changed to say HTTPS.
  • Also in https://t.me/c/1161850314/4292, they removed limits for text length and caption. Should we do that too? Or just document saying that premium users' bots' can write longer captions?

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.

Only have a few comments for the test :)

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.

found out why docs were failing: Also see #3112 (comment)

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.

I think apart from the two comments below I don't have anything else to add :) extending customwebhookbot.py to use the secret_token can surely be done in this PR but can also be done later on - might be another good first issue …

Poolitzer and others added 2 commits June 23, 2022 09:10
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
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.

LGTM

@Poolitzer Poolitzer mentioned this pull request Jun 24, 2022
async def test_premium_animation(self, bot):
# testing animation sucks a bit since we can't create a premium sticker. What we can do is
# get a sticker set which includes a premium sticker and check that specific one.
premium_sticker_chat = await bot.get_sticker_set("Flame")
Copy link
Member

Choose a reason for hiding this comment

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

_set instead of _chat?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

That's my point: premium_sticker_chat -> premium_sticker_set ;P

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohh you meant the variable I only saw the request

@Poolitzer Poolitzer mentioned this pull request Jun 25, 2022
Poolitzer and others added 2 commits June 25, 2022 15:52
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
@Bibo-Joshi Bibo-Joshi merged commit 08e223b into master Jun 27, 2022
@Bibo-Joshi Bibo-Joshi deleted the api_6.1 branch June 27, 2022 16:54
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2022
@harshil21 harshil21 modified the milestones: v20.0a3, v20.0a2 Jul 23, 2022
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.

3 participants