Skip to content

Conversation

@s-chizhik
Copy link

Described in #666

@jh0ker
Copy link
Member

jh0ker commented Jun 18, 2017

This looks pretty good, thank you :) I think we can simply ignore the ConversationHandler for this and say that you can either change state OR stop propagation. However, I would like to see some unit tests (similar to what you had in #666), in test_updater.py I guess.

@tsnoam tsnoam added the 📋 pending-reply work status: pending-reply label Jun 20, 2017
@tsnoam tsnoam requested a review from jh0ker June 20, 2017 19:42
@s-chizhik
Copy link
Author

Ok, so we want to check, that StopPropagetion exception won't change ConversationHandler state, correct? I'll send new changes asap :)

@tsnoam
Copy link
Member

tsnoam commented Jul 28, 2017

@s-chizhik
We are aware that your PR was here first, however #738 covers an additional scenario and has unitests.
In general you've both came to a similar solution which kinda suggests that this is going in the right direction.
If you are still interested in seeing this integrated, I suggest that you join forces with @ihoru and work on top of #738.
Therefor I am closing this PR.

@tsnoam tsnoam closed this Jul 28, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📋 pending-reply work status: pending-reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants