[dev] [commits] Horde branch master updated. 3eb04cb5d8972b124aed95ed568d056c8e980dc3

[Michael M Slusarz]

Quoting Jan Schneider <jan at horde.org>:

>> commit 3eb04cb5d8972b124aed95ed568d056c8e980dc3
>> Author: Michael M Slusarz <slusarz at horde.org>
>> Date:   Mon Feb 6 15:32:05 2012 -0700
>>
>>    Add Horde_Core_Alarm_Handler_Notify class
>>
>>    The injector passed to the Alarm Notify class requires that create()
>>    have no arguments.  However, the factory class requires that create()
>>    have a Horde_Injector argument.
>>
>>    The alternative is to create a wrapper object that wraps the create()
>>    method and adds the argument.  But at that point, we might as well
>>    create a local Horde_Core driver.
>>
>>    Wondering if we should do this for the Mail and Desktop drivers also.
>>
>> framework/Core/lib/Horde/Core/Alarm/Handler/Notify.php |   79  
>> ++++++++++++++++
>> framework/Core/lib/Horde/Core/Factory/Alarm.php        |    8 +-
>> framework/Core/package.xml                             |   12 ++-
>> 3 files changed, 91 insertions(+), 8 deletions(-)
>> create mode 100644 framework/Core/lib/Horde/Core/Alarm/Handler/Notify.php
>>
>> http://git.horde.org/horde-git/-/commit/3eb04cb5d8972b124aed95ed568d056c8e980dc3
>
> To me it looks like the original intention was to have  
> Horde_Core_Factory_Notification extend Horde_Core_Factory_Base so  
> that create() does indeed not require a parameter. Is there anything  
> speaking against this?

That would be one solution, but adding another Factory with a  
different injector style just to pass to the Alarm Handler is more  
confusing than just moving the essentially Horde-specific Alarm  
handler to Core.

The other 2 Alarm handlers are probably even better candidates to move  
to Core.  And I doubt the Desktop handler even works since it uses  
javascript notification handling which was deprecated long ago.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]