By the same logic, checkstyle is a huge waste of time and effort. As is
paying attention to deprecations.

If you have failed to see findbugs find a bug worth fixing, then it is
because you refuse to use it and don't pay attention to its output. I have
seen at least a half a dozen real bugs discovered by findbugs on FOP, and
I'm sure there are likely others being masked by the current excludes file
which bear further investigation.

I'm going to enable it for nightly builds, and hopefully you will join the
rest of us in using it.



On Mon, Jun 16, 2014 at 10:02 AM, Vincent Hennebert <vhenneb...@gmail.com>
wrote:

> On 16/06/14 16:10, Glenn Adams wrote:
>
>> Given that, with the exception of yourself, the findbugs exclude file has
>> been continually updated to produce no warnings, I conclude that the FOP
>> dev ha implicitly or explicitly accepted it for quite some time.
>>
>
> And yet, consensus has not been reached on this, since at least one
> developer is not keen on using it.
>
> If there were a real benefit in using FindBugs, then I would be the
> first one to promote its use. But that’s not the case, at least not in
> the present state.
>
> Quite on the contrary, I’ve seen bad things done to the code for the
> sake of keeping FindBugs quiet. Like surrounding whole chunks of code
> with checks for null when the variables are actually not meant to be
> null. Or instanceof checks when the variable can have only one type.
>
> Such checks seriously impede the readability of the code. Whoever comes
> after that will wonder why there is a check for null, and why nothing is
> done when the variable is null.
>
> And if that were only that. They also make debugging much harder by
> silently ignoring errors instead of making the code fail-fast.
>
> I have yet to see cases where I think: “Oh yes that’s right, there was
> a serious bug here, thanks FindBugs for letting me know.” In most cases,
> it’s rather: “Naah, that’s not an error, I’m going to have to update
> this exclude file again, which pattern should I put again, where’s the
> page that lists them, ah here it is, oh dear, which one is it among
> those hundreds?” And that’s to remain polite :-)
>
>
>
>  We don't need to do anything to the excludes file to make it acceptable.
>> It
>> is acceptable now. That doesn't mean it couldn't be improved by additional
>> review and revision, but that is true of the entire FOP code base.
>>
>
> I hope the above shows why it’s not acceptable as it is. At the moment
> and in my opinion, running FindBugs is a waste of time and energy rather
> than a help.
>
>
> Vincent
>
>
>
>  On Mon, Jun 16, 2014 at 3:32 AM, Vincent Hennebert
>> <vhenneb...@gmail.com>
>> wrote:
>>
>>  Rather than repeating myself, I’ll just point to the past conversations
>>> on this topic:
>>> http://markmail.org/message/pg7p2khityg4bjya
>>> http://markmail.org/message/taztvj2ms7x6tryb
>>>
>>> This one is particularly interesting:
>>> http://markmail.org/message/lthcstbbmhtbrsva
>>> It more or less matches my own experience with FindBugs.
>>>
>>> The only way to ensure zero FindBugs warnings is to enforce it in
>>> continuous integration, just like we did about Checkstyle. But before
>>> doing that, you’ll have to come up with a satisfying exclude file that
>>> filters out uninteresting warnings, and get the community to agree on
>>> it.
>>>
>>> Vincent
>>>
>>>
>>>
>>> On 15/06/14 17:03, Glenn Adams wrote:
>>>
>>>  I'm getting tired of fixing findbugs warnings that people are leaving in
>>>> the
>>>> code base. I'm going to start inserting comments that will break
>>>> compiles/builds if people don't check and fix these. Today I see four
>>>> new
>>>> warnings:
>>>>
>>>> *Dodgy Warnings*
>>>>
>>>> *Code*
>>>>
>>>>
>>>>
>>>> *Warning*
>>>>
>>>> *BC*
>>>>
>>>>
>>>>
>>>>
>>>> Unchecked/unconfirmed cast from org.apache.fop.fo.FONode to
>>>> org.apache.fop.fo.flow.MultiCase in
>>>> org.apache.fop.layoutmgr.LayoutManagerMapping$
>>>> MultiCaseLayoutManagerMaker.make(FONode,
>>>> List)
>>>>
>>>>
>>>>
>>>>
>>>> *BC*
>>>>
>>>>
>>>>
>>>>
>>>> Unchecked/unconfirmed cast from org.apache.fop.fo.FONode to
>>>> org.apache.fop.fo.flow.MultiSwitch in
>>>> org.apache.fop.layoutmgr.LayoutManagerMapping$
>>>> MultiSwitchLayoutManagerMaker.make(FONode,
>>>> List)
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> *BC*
>>>>
>>>>
>>>>
>>>>
>>>> Unchecked/unconfirmed cast from org.apache.fop.fo.FObj to
>>>> org.apache.fop.fo.flow.MultiSwitch in new
>>>> org.apache.fop.layoutmgr.MultiSwitchLayoutManager(FObj)
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> *RCN*
>>>>
>>>>
>>>>
>>>>
>>>> Redundant nullcheck of footnoteList, which is known to be non-null in
>>>> org.apache.fop.layoutmgr.list.ListItemLayoutManager.
>>>> getCombinedKnuthElementsForListItem(List,
>>>> List, LayoutContext)
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>

Reply via email to