Bots: Add default (and other) command dispatch #169
Conversation
| def __str__(self): | ||
| return self.msg | ||
|
|
||
| to_dispatch = {} |
There was a problem hiding this comment.
This doesn't feel right, having to_dispatch being a module-level variable.
There was a problem hiding this comment.
Would you prefer this in eg. ExternalBotHandler? Or in a separate class?
Is this approach preferable to #66?
|
Heads up @neiljp, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Also tests using new framework.
Extend the dispatch_default_commands function to take an 'other_commands' parameter, in order to support listing commands which are not just those listed as default.
To support the help text output, the other_commands parameter to dispatch_default_commands is now a mapping, and the internal supported_commands list is now an OrderedDict.
Extend tests accordingly.
neiljp
force-pushed
the
command_registry
branch
from
5bb8f26 to
1913dac
Compare
|
Heads up @neiljp, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
This is an alternative implementation of #66 based on discussion in #integrations regarding perceived issues (@showell, @timabbott)
I avoided amending existing bots and instead added a demonstration as a 'helloworld_defaults' bot, which demonstrates the increasing features in subsequent commits:
I attempted to develop things in tandem in the test system, but am not fully satisfied with the current system (as per the comments in test_helloworld_defaults).
There is no doc.md file as yet for the new bot, though I can add this easily.
I'm completely open to renaming aspects of the system, but functionally it appears to work fine in tests and with zulip_bot_output.py (or did until I rebased, and now have some kind of storage error? @roberthoenig?)