[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