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

Fallback fixes #26

Merged
merged 3 commits into from
Jan 1, 2021
Merged

Fallback fixes #26

merged 3 commits into from
Jan 1, 2021

Conversation

sergix44
Copy link
Collaborator

@sergix44 sergix44 commented Jan 1, 2021

Apply some fixes to the latest addition: if a listener if found for callback queries, no need to call the fallback method, and all the global middlewares should be applied to the fallback listener.

@cheeghi
Copy link
Collaborator

cheeghi commented Jan 1, 2021

Ok rievewed and seems fine, so fallback listeners are meant to be triggered only on text messages (like commands), right?

@sergix44
Copy link
Collaborator Author

sergix44 commented Jan 1, 2021

Ok rievewed and seems fine, so fallback listeners are meant to be triggered only on text messages (like commands), right?

I've changed a bit the logic and added some more comments to explain whats going on, I hope it will be a bit more clear.
You are right, in certain condition calling the fallback also for cb queries can be useful, tell me what you think abou the new changes

@cheeghi
Copy link
Collaborator

cheeghi commented Jan 1, 2021

Yeah, I think it's more clear. For cb queries I was just wondering actually 😅 I see the fallback usefulness in texts (eg. if someone sends a command that doesn't exist), less in cb queries, but why not including them. So, for me it's fine as is!

@sergix44
Copy link
Collaborator Author

sergix44 commented Jan 1, 2021

Yeah, should be fine, hoping for the best!

@cheeghi cheeghi merged commit 56845bd into badfarm:develop Jan 1, 2021
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.

None yet

2 participants