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

Ordered commands #101

Merged
merged 1 commit into from
Dec 9, 2016
Merged

Ordered commands #101

merged 1 commit into from
Dec 9, 2016

Conversation

gconklin
Copy link

@gconklin gconklin commented Nov 7, 2016

My attempt at addressing #75 -- probably not the most ruby way to do it but is working for my purposes; comments welcome.

@dblock
Copy link
Collaborator

dblock commented Nov 7, 2016

It's an interesting solution. However I think it would be simpler to keep a global list of commands when inherited and not rely on dynamically accessing SlackRubyBot::Commands::Base.descendants.

I would start by writing a test :)

@gconklin
Copy link
Author

gconklin commented Nov 7, 2016

I am king of Interesting Solutions(tm). I will start by writing a test! as i should have.

Commands order slack-ruby#75 notes

maintain an array of registered command classes
@dblock
Copy link
Collaborator

dblock commented Dec 9, 2016

Just noticing this @gconklin, merging.

@dblock dblock merged commit aefdc02 into slack-ruby:master Dec 9, 2016
@dblock
Copy link
Collaborator

dblock commented Feb 12, 2017

This caused #113. I fixed it in a straightforward way in #114, but I am not sure it's the right way. Would appreciate a second pair of eyes from @gconklin, please.

@gconklin
Copy link
Author

gconklin commented Feb 12, 2017

I spent some time looking. It does clearly fix #113, but after a bit of Googling, I'm still not entirely sure why @command_classes is different from SlackRubyBot::Commands::Base.command_classes in the context of self. I expect it's my failure to grasp class instance variables, and that @command_classes is different when the test bot inherits from SlackRubyBot::Bot as opposed to a command directly inheriting from SlackRubyBot::Commands::Base
@dblock

@dblock
Copy link
Collaborator

dblock commented Feb 13, 2017

I couldn't figure it out either, no worries.

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.

None yet

2 participants