CommandsHelper class and Help module refactoring #182
Conversation
|
@dblock I took into account your comment about I think we can cut a release once we agree on this refactoring. I can also take a look into rest of your suggestions before we do this to avoid public interface changes later. |
CHANGELOG.md
Outdated
| @@ -1,6 +1,7 @@ | |||
| ### 0.10.6 (Next) | |||
|
|
|||
| * Your contribution here. | |||
| * [#182](https://github.com/slack-ruby/slack-ruby-bot/pull/182): CommandsHelper class and Help module refactoring - [@mdudzinski](https://github.com/mdudzinski). | |||
There was a problem hiding this comment.
Sounds better as Refactored ...
There was a problem hiding this comment.
Sure, I'll change it.
There was a problem hiding this comment.
Actually, it should be Refactor ... as commit title/summary should be imperative. Unless you want the pr's title to be different than the commit's one.
lib/slack-ruby-bot/commands.rb
Outdated
| @@ -1,5 +1,5 @@ | |||
| require 'slack-ruby-bot/commands/base' | |||
| require 'slack-ruby-bot/commands/about' | |||
| require 'slack-ruby-bot/commands/help' | |||
| require 'slack-ruby-bot/commands/help_command' | |||
There was a problem hiding this comment.
This one should be help, just like about or hi, not _command, no?
There was a problem hiding this comment.
Well, I didn't want it to be confusing with SlackRubyBot::Commands::Help from commands/support. And the class name is HelpCommand so I thought its file should match that name.
There was a problem hiding this comment.
I'm just thinking either all commands are SomethingCommand or Something. Thoughts?
There was a problem hiding this comment.
Just Something seems to be better but then the help command's class should be renamed to Help and we'll have to change the name of Help class in support. Or add a namespace like SlackRubyBot::Commands::Support::Help.
There was a problem hiding this comment.
I like the namespace idea.
There was a problem hiding this comment.
@dblock Okay I'll change it. If there's anything else, let me know. Thanks :)
The 'Help' namespace was renamed to `Support` and the 'help' directory was renamed to 'support'. Since 'CommandHelper' was a confusing utility name (it just handles commands help), it was renamed to 'Help' and moved to 'commands/support' directory. Now its full name is 'SlackRubyBot::Commands::Support::Help'.
|
@dblock Updated, please take a look. Thanks! |
|
Perfect, merged. |
This is a PR related to the refactoring discussion under #180.