nutch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Piotr Kosiorowski <pkosiorow...@gmail.com>
Subject PMD integration (was: Re: Add ".settings" to svn:ignore on root Nutch folder?)
Date Thu, 06 Apr 2006 19:24:30 GMT
Hi,
I have downloaded the patches and generally like them (I had only read 
them not applied yet). I have one question - am I reading it correctly 
that right now it is checking only main code (without plugins?).
P.


Dawid Weiss wrote:
> 
> All right, I though I'd give it a go since I have a spare few minutes. 
> Jura is off, so I made the patches available here --
> 
> http://ophelia.cs.put.poznan.pl/~dweiss/nutch/
> 
> pmd.patch is the build file patch and libraries (binaries are in a 
> separate zip file pmd-ext.zip).
> 
> pmd-fixes.patch fixes the current core code to go through pmd smoothly. 
> I removed obvious unused code, but left FIXME comments where I wasn't 
> sure if the removal can cause side effects (in these places PMD warnings 
> are suppressed with NOPMD comments).
> 
> I also discovered a bug in PMD... eh... nothing's perfect.
> 
> https://sourceforge.net/tracker/?func=detail&atid=479921&aid=1465574&group_id=56262

> 
> 
> D.
> 
> 
> Piotr Kosiorowski wrote:
>> +1 - I offer my help - we can coordinate it and I can do a part of 
>> work. I
>> will also try to commit your patches quickly.
>> Piotr
>>
>> On 4/6/06, Dawid Weiss <dawid.weiss@cs.put.poznan.pl> wrote:
>>>
>>>> Other options (raised on the Hadoop list) are Checkstyle:
>>> PMD seems to be the best choice for an Apache project and they all seem
>>> to perform at a similar level.
>>>
>>>> Anything that generates a lot of false positives is bad: it either
>>>> causes us to skip analysis of lots of files, or ignore the warnings.
>>>> Skipping the JavaCC-generated classes is reasonable, but I'm wary of
>>>> skipping much else.
>>> I thought a bit about this. The warnings PMD may actually make sense to
>>> fix. Take a look at maxDoc here:
>>>
>>> class LuceneQueryOptimizer {
>>>
>>>    private static class LimitExceeded extends RuntimeException {
>>>      private int maxDoc;
>>>      public LimitExceeded(int maxDoc) { this.maxDoc = maxDoc; }
>>>    }
>>> ...
>>>
>>> maxDoc is accessed from LuceneQueryOptimizer which requires a synthetic
>>> accessor in LimitExceeded. It also may look confusing because you
>>> declare a field private to a class, but use it from the outside...
>>> changing declarations to something like this:
>>>
>>> class LuceneQueryOptimizer {
>>>
>>>    private static class LimitExceeded extends RuntimeException {
>>>      final int maxDoc;
>>>      public LimitExceeded(int maxDoc) { this.maxDoc = maxDoc; }
>>>    }
>>> ...
>>>
>>> removes the warning and also seems to make more sense (note that package
>>> scope of maxDoc doesn't really expose it much more than before because
>>> the entire class is private).
>>>
>>> So... if you agree to change existing warnings as shown above (there's
>>> not that many) then integrating PMD with a set of sensible rules may
>>> help detecting bad smells in the future (I couldn't resist -- it really
>>> is called like this in software engineering :). I only used dead code
>>> detection ruleset for now, other rulesets can be checked and we will see
>>> if they help or quite the contrary.
>>>
>>> If developers agree to the above I'll create a patch together with what
>>> needs to be fixed to cleanly compile. Otherwise I see little sense in
>>> integrating PMD.
>>>
>>> D.
>>>
>>>
>>>
>>>
>>
> 


Mime
View raw message