Remove DispatcherHandlerContinue + more unitests for dispatcher #792
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The idea was nice, but it really complicated things for us and for the user. If a user wants to run more than one handler on an update, he can put the handlers in different groups or he can have a single handler. If a user wants to have multiple handlers in the same group which only one of them should run on the update, he should use check_update(). Since we haven't released this code yet, there's no problem with backward compatability.
Eldinnie
reviewed
Aug 12, 2017
| def add_handler(self, handler, group=DEFAULT_GROUP): | ||
| """ | ||
| Register a handler. | ||
| """Register a handler. |
Member
There was a problem hiding this comment.
I moved all docstrings to a newline after """ in all files. Can you please keep that here?
Member
Author
There was a problem hiding this comment.
as discussed, i followed pep8
telegram/ext/dispatcher.py
Outdated
| except situations when :class:`telegram.DispatcherHandlerContinue` or | ||
| :class:`telegram.DispatcherHandlerStop` were raised. | ||
| evaluated for handling an update, but only 0 or 1 handler per group will be used. If | ||
| :class:`telegram.DispatcherHandlerStop` is raised from one of the handler, no further |
| which handlers were added to the group defines the priority. | ||
| * If :class:`telegram.DispatcherHandlerContinue` was raised, then next handler in the | ||
| same group will be called. | ||
| * If :class:`telegram.DispatcherHandlerStop` was raised, then zero handlers (even |
Member
There was a problem hiding this comment.
I think the bullet for DispatcherHandlerStop is still valid in the docstring?
Member
Author
There was a problem hiding this comment.
it's kind of duplicate of the paragraph above, don't you think?
Member
There was a problem hiding this comment.
maybe yes, but now it looks weird as a bullet list for order with just one bullet
Member
Author
There was a problem hiding this comment.
two bullets. the code below this patch has a second bullet...
Eldinnie
approved these changes
Aug 12, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea was nice, but it really complicated things for us and for the
user.
If a user wants to run more than one handler on an update, he can put
the handlers in different groups or he can have a single handler.
If a user wants to have multiple handlers in the same group which only
one of them should run on the update, he should use check_update().
Since we haven't released this code yet, there's no problem with
backward compatability.