Hi Simon,

On 23/02/11 07:51, Simon Pepping wrote:
> On Tue, Feb 22, 2011 at 07:15:17PM +0000, Vincent Hennebert wrote:
>> On 22/02/11 07:24, Simon Pepping wrote:
>>> Not all FOP developers are willing to use findbugs. I hid the findbugs
>>> errors as a courtesy to those FOP developers who do use findbugs, so
>>> they can check their own code based on a clean slate.
>>
>> I agree that ignoring all the existing issues at the time FindBugs was
>> set up was necessary, but now that FindBugs is in place I don’t think we
>> want to blindly ignore its output any more. Again, some of the issues
>> raised after the recent commits seem valid and ought to be fixed.
>>
>> Can we revert commit 1071912 and re-consider the issues one-by-one
>> before ignoring them?
> 
> Yes, you can. 
> 
> My position is as follows: I do merging of trunk into complexscripts
> as a service to the Complex Scripts project. I see existing findbugs
> errors and warnings. I do not have time nor interest to fix those. So
> I hide them so that others can run findbugs on their code from a clean
> slate. I add a new, appropriately labeled section to the findbugs
> exclusion file so that you and others can do as you suggest.
> 
> I will do this again and again, no matter how many emails are written
> about the subject. And I would prefer it if no emails about it were
> written. They bother me and they discourage me to continue doing any
> services for FOP.

Are you bothered because you think the value of your services isn’t
fully recognized? I think nobody here would question your contributions
and it’s great to have you on board. I think what we’re discussing here
is just the approach we want to take regarding FindBugs, that hasn’t
been fully defined yet.

May I suggest you to not bother about FindBugs at all? Now that you’ve
set up the initial exclude file, it produces useful output that isn’t
lost among the many warnings and errors coming from the legacy code
base. I think that from now on we want to encourage committers to run
FindBugs prior to committing code, fix new issues raised by it, or make
an informed decision to ignore them.


> It is much more encouraging to see that some of us
> pick up the findbugs error report and use it to make code
> improvements.

Well it’s certainly great to see dedicated committers fixing FindBugs
issues that don’t originate from their work. For legacy code this is
what we will have to do as we go along. For new code however, I find
that hugely unfair. I think every committer should take the
responsibility to check their own code. I see this as common courtesy
towards other committers and contributors.

Do we all agree to run FindBugs along with JUnit prior to every commit
from now on?


Thanks,
Vincent


> Simon
> 
>>> FOP's history has left us with a very large number of existing
>>> findbugs errors. It makes no sense to comment on that; it is a fact of
>>> life. FOP's code first of all does a good job at being a successful,
>>> heavily used FO processor. Only after that comes the question of a
>>> clean code base.
>>>
>>> That said, of course every FOP developer and every FOP user is welcome
>>> to evaluate and possibly remedy one or more findbugs errors. For
>>> precisely that reason I put comments in the exclusion file, so one can
>>> see which errors have been simply hidden and which have been truely
>>> evaluated.
>>>
>>> Simon
>>
>> Thanks,
>> Vincent
>>
>>
>>> On Mon, Feb 21, 2011 at 06:15:13PM +0000, Vincent Hennebert wrote:
>>>> Hi,
>>>>
>>>> If we solve issues raised by FindBugs by listing them in an ignore file,
>>>> is there still a point to use FindBugs at all?
>>>>
>>>> It seems to me that some of those issues deserve to be fixed. They seem
>>>> to point out genuine problems in the code.
>>>>
>>>> Vincent
>>>>
>>>>
>>>> On 18/02/11 08:18, spepp...@apache.org wrote:
>>>>> Author: spepping
>>>>> Date: Fri Feb 18 08:18:04 2011
>>>>> New Revision: 1071912
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1071912&view=rev
>>>>> Log:
>>>>> Fixing checkstyle errors and hiding fingbugs errors
>>>>>
>>>>> Modified:
>>>>>     xmlgraphics/fop/trunk/findbugs-exclude.xml
>>>>>     
>>>>> xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
>>>>>
>>>>> Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/findbugs-exclude.xml?rev=1071912&r1=1071911&r2=1071912&view=diff

Reply via email to