Implementation of targeted reminders. #246

Merged
merged 4 commits into from Jan 25, 2013

Conversation

Projects
None yet
2 participants
Contributor

Drarok commented Jan 23, 2013

Add the ability for Phergie to only respond to targeted messages. That is, ones prefixed with the bot name. Refs #245.

Owner

elazar commented Jan 24, 2013

Thank you for your pull request.

This change introduces a soft dependency on the Message plugin. It should either be checked for using $this->plugins->hasPlugin('Message') shortly before it's used or it should be added as a hard dependency in the class docblock and in onLoad(). While an indirect dependency already exists because Remind requires Command and Command requires Message, without this change, that fact is transparent to Remind and it's best that the assumption that this relationship will always exist not be codified.

Please amend your PR to include the above changes so it can be merged. Your contribution is appreciated.

Contributor

Drarok commented Jan 24, 2013

I've added it as a hard dependency only when the option is set, leaving it out otherwise. :)

Edit: Also, rebased onto master since my other PR conflicted with this one.

Owner

elazar commented Jan 24, 2013

Can you please also add this to the class docblock?

 @uses Phergie_Plugin_Remind pear.phergie.org optional

This will add to the built PEAR package such that PEAR will prompt the user to install Remind as an optional dependency.

Contributor

Drarok commented Jan 24, 2013

Easy peasy. :)

@elazar elazar added a commit that referenced this pull request Jan 25, 2013

@elazar elazar Merge pull request #246 from Drarok/drarok/targeted-reminders
Implementation of targeted reminders.
374618c

@elazar elazar merged commit 374618c into phergie:master Jan 25, 2013

Owner

elazar commented Jan 25, 2013

Thanks for your contribution!

Drarok deleted the Drarok:drarok/targeted-reminders branch Jan 25, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment