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

Unknown command #489

Closed
wants to merge 1 commit into from
Closed

Conversation

lewisjb
Copy link
Collaborator

@lewisjb lewisjb commented Oct 10, 2019

Fixes #471

Copy link
Member

@kumar-ish kumar-ish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, just get Jenkins' approval

if not handlers:
self.post_message(command.channel_id, "Unknown command. Use !help to retrieve a list of supported commands")
else:
for handler in self._command_registry[command.name]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this for handler in handlers:? A bit more elegant, is all.

self.executor.submit(self._execute_catching_error, handler, command)
handlers = self._command_registry[command.name]
if not handlers:
self.post_message(command.channel_id, "Unknown command. Use !help to retrieve a list of supported commands")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split up the String argument onto the next line so that Jenkins doesn't disapprove. It currently says

uqcsbot/base.py:198:101: E501 line too long (120 > 100 characters)

@TRManderson TRManderson force-pushed the master branch 2 times, most recently from 354d58e to 2fbec70 Compare January 3, 2020 05:01
@nicklambourne
Copy link
Member

@lewisjb I'm just cleaning up these PRs, are you able to make the suggested changes and merge master from upstream?

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.

Unknown Command
3 participants