Skip to content
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

chg: allow callback_query_handler to do not have func arg. #2345

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

hiddify-com
Copy link
Contributor

Description

when we have a custom filter forcing using func is not acceptable in callback_query_handler so i have removed it

Describe your tests

How did you test your change?

Python version:

OS:

Checklist:

  • [*] I added/edited example on new feature/change (if exists)
  • [*] My changes won't break backward compatibility
  • [*] I made changes both for sync and async

@Badiboy
Copy link
Collaborator

Badiboy commented Jul 12, 2024

when we have a custom filter forcing using func is not acceptable in callback_query_handler so i have removed it

Very (extremely) breaf reasoning. We (at least I) require detailed description of:

  1. What for
  2. How it will be used (with examples)
  3. Proof that it will not break the backward compatibility

@coder2020official
Copy link
Collaborator

Hi, this is actually a useful PR. Let me talk instead:

  1. Func should not be a required parameter. After implementing custom filters, many use them and hence don't rely on func for obvious reasons. In addition, some handlers already have func as optional.
  2. @bot.callback_query(data=...) use a filter with data key instead of lambda function
  3. Making a parameter optional shouldn't break the compatibility. Those who already pass values to it won't suffer, and those already setting it to None won't either. Filters are checked(telling this from my memory) and filter is discarded if it is None.

Feel free to quote me

@Badiboy
Copy link
Collaborator

Badiboy commented Jul 12, 2024

No problem. If you (@coder2020official ) approve it - it's ok.

@coder2020official
Copy link
Collaborator

merge @Badiboy

@Badiboy Badiboy merged commit f91c618 into eternnoir:master Jul 17, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants