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

[Gunnar Wrobel]

Quoting Chuck Hagenbuch <chuck at horde.org>:

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

Fixed.

> and what we already said about decorator naming.

Interfaces and Decorators have been renamed now.

> And a random detail:
>
> +    public function setNotificationListeners(array &$options);
>
> Why a reference there? If $options is being changed, shouldn't it be  
> returned instead?

Probably clearer that way. Modified that.


I tested the branch with kronolith which worked fine. So I merged it now.

I am however unable to delete the branch in the horde repository.

# git push horde-write :refs/heads/refactor-Notification
error: denying ref deletion for refs/heads/refactor-Notification
To ssh://dev.horde.org/horde/git/horde
  ! [remote rejected] refactor-Notification (deletion prohibited)
error: failed to push some refs to 'ssh://dev.horde.org/horde/git/horde'

Is that intentional?

Cheers,

Gunnar


>
>
> Otherwise, I'm on board with this going forward and being merged. Thanks!
>
> -chuck
>
> -- 
> Horde developers mailing list - Join the hunt: http://horde.org/bounties/
> Frequently Asked Questions: http://horde.org/faq/
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org
>




-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digitale PGP-Unterschrift
URL: <http://lists.horde.org/archives/dev/attachments/20091105/910314ed/attachment.bin>