[dev] [commits] Horde branch refactor-Notification updated. 3b590095a4312f85639071510adc42d5dc9090fc

[Chuck Hagenbuch]

Quoting Gunnar Wrobel <p at rdus.de>:

> Hi,
>
> as I need to use the Horde_Notification package in stand-alone mode  
> I was forced to refactor it a bit. The main problem was that the  
> module is firmly embedded into the global Horde context and contains  
> a number of calls to services such as Horde_Registry, Horde_Auth,  
> Horde_Nls, Horde::logMessage(). These service are of course missing  
> if the package is used on its own.
>
> I pushed the necessary refactorings into the "refactor-Notification"  
> branch. It would be nice if I could get some feedback on the  
> changes. Do they work or are there conflicts? Does the structure  
> look okay? Can I merge the branch?
>
> The main changes were two new components: The  
> Horde_Notification_Storage and the Horde_Notification_Handler  
> interfaces. The storage part allows to exchange the underlying  
> storage mechanism (for Horde this is the session by default). And  
> the handler part moves the references to global scope that were  
> present in Horde_Notification into separate decorators. This way  
> they can be avoided when using the Horde_Notification for itself.

High-level, your changes sound good. For specifics, Jan's note about  
alarms, and what we already said about decorator naming. And a random  
detail:

+    public function setNotificationListeners(array &$options);

Why a reference there? If $options is being changed, shouldn't it be  
returned instead?


Otherwise, I'm on board with this going forward and being merged. Thanks!

-chuck