[dev] CI suggestion

Michael M Slusarz slusarz at horde.org
Thu Feb 24 21:32:05 UTC 2011


Quoting Gunnar Wrobel <wrobel at horde.org>:

> Hi Micheal,
>
> Zitat von Michael M Slusarz <slusarz at horde.org>:
>
>> CI/Jenkins is a really cool tool (Thanks Gunnar!)  But one  
>> suggestion: ditch the code complexity tests.  They are worthless.   
>> They clutter the output for the more useful results, and don't  
>> provide any meaningful input.  They would have you reduce a  
>> method/class to the point of extreme abstraction, and  
>> unmaintainable code, just for the sake of making sure an artificial  
>> number stays below a certain vague threshold.
>>
>> Either that or dramatically increase the limits before it starts  
>> throwing warnings.  E.g., it complains that Horde_Variables "has  
>> too many methods".  Really?  It has 22: 1 is the constructor, 4 are  
>> magic methods for property manipulation, 5 are methods requiring to  
>> implement Iterator, and 1 is a method required to implement  
>> Countable.  I hardly think the 11 remaining methods in this class  
>> are "too many".  (I don't think 100 methods in a class is too many  
>> either, FWIW, as long as they coherently tie in with each other).
>
> To be honest I didn't care very much about these numbers yet. When I  
> originally considered continuous integration for Horde I really  
> pondered on having just a script that would run  
> framework/bin/test_framework and mail to the list if there were  
> problems. Jenkins comes with more bells and whistles and I won't  
> disagree that it is a tad too much.
>
> The reason why I still favored the fluffy, colored approach is  
> because it is close to what others try to establish as a standard  
> for continuous integrations setups in the PHP world  
> (http://jenkins-php.org/). In my experience people in the business  
> world do actively look for things like that when they consider using  
> external open source libraries for their own development. So if  
> possible I'd like to stick close to that "standard" in order to  
> indicate that we are interested in what others consider to be good  
> coding practices. Completely ditching PMD might be a bit much.

Not asking to ditch PMD.  Just asking to remove/tweak some of those tests.

> At the same time I agree that we probably don't want to look bad on  
> http://ci.horde.org. And if there are PMD rules we know don't care  
> for I don't see any reason why we shouldn't remove them from the  
> ruleset. We did define our own Horde-specific code style definition  
> for the CodeSniffer as well. So we can certainly also define our own  
> ruleset for PMD to suit our needs.

That would be what I recommend.  I don't have an issue with keeping  
the tests - we just need to set the triggers WAY higher IMHO.

> It is quite some time ago since I looked at the PMD ruleset  
> definition - I think it was just an XML configuration. But it is  
> probably nothing I will get into before we had our Horde 4 release.  
> If others have an alternative definition I'm fine with replacing the  
> default.

I'd be willing to look at this if I find the time.  But I probably  
would forward recommended changes to you since I am not familiar/don't  
want to break the current system.

michael

-- 
___________________________________
Michael Slusarz [slusarz at horde.org]




More information about the dev mailing list