Skip to content

Improve signature of shortcuts#2240

Merged
Bibo-Joshi merged 18 commits intomasterfrom
fix-2238
Dec 30, 2020
Merged

Improve signature of shortcuts#2240
Bibo-Joshi merged 18 commits intomasterfrom
fix-2238

Conversation

@Bibo-Joshi
Copy link
Copy Markdown
Member

@Bibo-Joshi Bibo-Joshi commented Dec 9, 2020

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
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

Bibo-Joshi commented Dec 20, 2020

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
Copy Markdown
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

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

@Poolitzer Poolitzer Poolitzer approved these changes

+1 more reviewer

@blueset blueset blueset left review comments

Reviewers whose approvals may not affect merge requirements

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