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

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> 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.

Yes, that won't make sense, I was rather thinking of replacing the  
existing factory. I don't think we call create() on Factory_Base  
factories explicitly anyway.

> 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.

Can you explain why? I don't see any reason why those shouldn't work  
just fine in a plain Horde_Notification installation (once the Desktop  
handler is fixed).

-- 
The Horde Project
http://www.horde.org/