Skip to content

Improve signature of shortcuts #2240

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Dec 30, 2020
Merged

Improve signature of shortcuts #2240

merged 18 commits into from
Dec 30, 2020

Conversation

Bibo-Joshi
Copy link
Member

First POC for #2238. It's probably good to discuss it first, before applying the changes to all the other shortcuts.

Some notes:

  • As per my comment on [ENHANCEMENT] Improve callback signature of shortcuts #2238, I skipped documentation of the arguments
  • manually passing all the arguments to the bot methods seemed tiresome to me and is also error prone (we would have to test passing aaaaall the args), so I came up with get_shortcut_kwargs, which can be used in combination with pythons built-in locals() method. It will probably have to get a argument local_vars_to_ignore for some of the shortcuts, but we'll see.
  • put the signature test (including annotations) in conftest.py to reduce code duplication in the tests
  • added Message._new_quote instead of editing Message._quote for now, so the test won't fail immediately …

when ready, closes #2238

@Bibo-Joshi Bibo-Joshi added enhancement 🛠 refactor change type: refactor labels Dec 9, 2020
@Bibo-Joshi Bibo-Joshi requested a review from Poolitzer December 9, 2020 15:42
Poolitzer
Poolitzer approved these changes Dec 16, 2020
View reviewed changes
Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

LGTM. Details in dev group

github-actions[bot]
github-actions bot reviewed Dec 17, 2020
View reviewed changes
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 there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

Poolitzer
Poolitzer reviewed Dec 17, 2020
View reviewed changes
Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

okay, looks good now. are we doing it like this?

@Bibo-Joshi Bibo-Joshi added this to the v13.2 milestone Dec 17, 2020
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Dec 17, 2020 •
edited
Loading

Here is a (hopefully complete) list of all TG classes with shortcuts. Put your name behind it, when you start working on it. that way we can keep track of who does what. Don't forget to update the tests ;P

  • CallbackQuery (@Bibo-Joshi )
  • Chat (@Poolitzer)
  • Message (@Bibo-Joshi )
  • Poll (doesn't have one yet, but we should add one for stop_poll, while at it actually that's BS, as Poll has acces to neither chat_id nor message_id, which are needed to stop it.)
  • User (@Bibo-Joshi )
  • InlineQuery (@Bibo-Joshi )
  • All the media types (Animation, Audio, ChatPhoto, Document, PhotoSize, Sticker, VideoNote, Video, Voice) @Bibo-Joshi
  • Passportfile (@Bibo-Joshi )
  • Precheckoutquery (@Bibo-Joshi )
  • Shippingquery (@Bibo-Joshi )

@Bibo-Joshi
Copy link
Member Author

TimeOut-failures seem unrelated and I think I got all the shortcuts. Ready for review.

@Bibo-Joshi Bibo-Joshi marked this pull request as ready for review December 20, 2020 11:44
@Bibo-Joshi Bibo-Joshi added the ⚙️ documentation affected functionality: documentation label Dec 20, 2020
@Bibo-Joshi Bibo-Joshi mentioned this pull request Dec 25, 2020
Closed
@blueset blueset mentioned this pull request Dec 25, 2020
Closed
Poolitzer
Poolitzer approved these changes Dec 30, 2020
View reviewed changes
Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

LGTM

@Bibo-Joshi Bibo-Joshi merged commit aec6d3b into master Dec 30, 2020
@Bibo-Joshi Bibo-Joshi deleted the fix-2238 branch December 30, 2020 12:41
@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2020
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blueset blueset blueset left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@Poolitzer Poolitzer Poolitzer approved these changes

Assignees
No one assigned
Labels
⚙️ documentation affected functionality: documentation 🔌 enhancement pr description: enhancement 🛠 refactor change type: refactor
Projects
None yet
Milestone
v13.2
Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Improve callback signature of shortcuts
3 participants
@Bibo-Joshi @blueset @Poolitzer