[dev] [commits] Horde branch master updated. 1e394986ba8dd83a6f2854f51c361f72fce9e7b5

[Chuck Hagenbuch]

Quoting Jan Schneider <jan at horde.org>:

> I'm not talking for Michael obviously, but I'm still gonna add my two cent.
>
> Zitat von Gunnar Wrobel <p at rdus.de>:
>
>> Hi Micheal,
>>
>> Quoting Michael M Slusarz <slusarz at horde.org>:
>>
>> [snip]
>>
>>>
>>> commit e48b8a54fc54a179be749967a6a8b6b73cc1162e
>>> Author: Michael M Slusarz <slusarz at curecanti.org>
>>> Date:   Mon Feb 15 23:27:31 2010 -0700
>>>
>>>   Another Notification rewrite.
>>>
>>>   First, fix Bug #8870. Fixed by removing Nag specific event type
>>>   (nag.alarm).  However, this was just a symptom of a larger problem.
>>>
>>>   The problem: using application specific Notification handlers to handle
>>>   application specific event types.  The problem comes when switching
>>>   between applications.  Since these application handlers don't have any
>>>   knowledge of each other, events created by one handler may not be able
>>>   to be displayed when notify() was eventually called, because another
>>>   status handler had replaced the original handler.
>>>
>>>   The solution: all notifications need to be handled by a single,
>>>   centralized source - namely, the horde-level handlers.  Application
>>>   specific details are instead injected into the horde-level handler to
>>>   extend behavior.
>>>
>>>   While reworking the code, also provided opportunity to remove all
>>>   application-specific code from Notification.  Horde-specific
>>>   instantiation (i.e. adding Horde logging and Alarm decorators) is now
>>>   done in Horde_Core rather than in the base Notification object.
>>
>> Cool, great! Thanks.
>>
>>>
>>>   Additionally, rework some of the complexity added to the package.  I
>>>   believe the goal of the recent Notification changes was to make the
>>>   Notification package testable and/or usable outside of a base Horde
>>>   install.
>>
>> Doing such a rewrite for testing purposes would seem wrong to me.  
>> The  only intention was to make the package usable outside of a  
>> standard  Horde installation.
>>
>> I hope it is okay to transform Horde framework packages this way.  
>> My  expectation has always been that the framework packages should  
>> work  that way and be as independent of each other as reasonable.  
>> Probably  originates from the fact that the Kolab server always  
>> relied to a  larger degree on the framework than on the  
>> applications. This has been  somewhat difficult in the past because  
>> of the tendency to use global  scope in the current code. But  
>> things are improving of course.
>
> This expectation is absolutely correct.
>
>> One question concerning the unit tests: I added them because they  
>> are  required to a certain degree by the project where I'm using  
>> the  Horde_Notification package. I am however not 100% certain if  
>> these  tests are useful to the Horde project. I see a tendency  
>> towards  PHPUnit testing within Horde. But we are also ignoring  
>> them to a  certain degree as we don't seem to care much if they  
>> break. And I feel  that at that point unit tests may become more of  
>> a nuisance rather  than being helpful.
>
> We rather need more tests than less. The fact that they break from  
> time to time is probably due to us not having used tests that much  
> in the past. We still have to get used to run them after each  
> change. Some continuous integration would probably help too, but I  
> don't see anyone having the resources at the moment to set this up.
>
>> Did the tests make sense to you while rewriting Horde notification  
>> or  did you consider them to be a hindrance?
>>
>>>   But these changes also made the code unreadable, redundant,
>>>   and overly complex.
>>
>> This sounds very negative which is why I am asking whether making  
>> the  packages more independant is actually okay.
>
> It's absolutely okay and should be forced even more.

I just want to echo Jan here. We are trying hard to make the framework  
packages useful libraries; this will hopefully benefit us just as much  
as others using them.

-chuck