No rationale, just an opinion:
FindBugs is a great tool for pointing to potential problems. But the
results are not black and white. Checkstyle is more black and white but
also not completely. Having no FindBug error doesn't mean the code works
and having a FindBug error doesn't mean there is a bug.

FindBugs can also not detect package access violations (ex. I've had to
revert multiple changes in the past that would have added a dependency
on the layout engine from inside the PDF library). Want to add a tool
for that and create a policy? What else? Hmm, what about test coverage
metrics? Hey, until 2005, FOP didn't even have a test infrastructure for
layout engine regressions. And we can't test our output files
automatically today. For certain output formats like AFP and PDF there
is a certain feasibility to build up a useful test coverage. But since
there's none, initial efforts would be rather high. Who allocates (or
can allocate) the time for that? To whom is it important enough? We'd
still be lacking automatic tests for the other output formats (PS, PCL,
bitmap...). IMO such tests would be much more helpful to FOP but they
are also damn hard to write.

Very strongly recommending that committers install a FindBug plug-in in
their IDE and activate it for FOP: yes. Establishing a zero-FindBugs
policy at build level: absolutely not.

Concerning Checkstyle:
Maintaining a zero checkstyle rule is ok when the rules allow for
reasonable leeway. At the moment, we still have the odd rule too many.
And I don't like the suppression/exclusion comments/file. This is
against the idea. Less is more. See also:
http://wingnut-diaries.tumblr.com/post/3314997105/source-code-formatting#disqus_thread
(to which I agree very much)

That said, I haven't seen any Apache project that has ever discussed
non-functional stuff like this to such an extent. I'm tired of these
discussions and it doesn't help my motivation to work here. Plus, the
entry hurdle for new contributions is raised ever higher which may scare
many away (as if FOP itself is not complicated enough already and many
patches long await processing). FOP can't afford that IMO. And I usually
don't get the budget to write rocket/nuclear-plant-level software.
Others probably don't, either.

On 25.02.2011 19:01:59 Glenn Adams wrote:
> ah, in that case, I wonder who would oppose a policy to use checkstyle
> and findbugs prior to commits in order to maintain a zero warnings
> state? and if opposed, then a rationale for that position?
> 
> /g
> 
> On Friday, February 25, 2011, Jeremias Maerki <d...@jeremias-maerki.ch> wrote:
> > It hasn't occurred, yet, but I expect one to happen if we were to
> > install a new policy. I'm just predicting a possible outcome. ;-)
> >
> > On 25.02.2011 16:57:25 Glenn Adams wrote:
> >> Did I miss seeing such a vote? Could you give me an approximate date when 
> >> it
> >> occurred so I can check the archives for background?
> >>
> >> Even if it failed to obtain consensus in the past, doesn't mean it might 
> >> not
> >> be accepted now.
> >>
> >> G.
> >>
> >> On Fri, Feb 25, 2011 at 4:32 AM, Jeremias Maerki 
> >> <d...@jeremias-maerki.ch>wrote:
> >>
> >> > Possibly a vote to establish such a policy falling through. Just a hunch.
> >> >
> >> > On 25.02.2011 08:09:11 Glenn Adams wrote:
> >> > > Well then, let's make it a policy. What's preventing that?
> >> > >
> >> > > On Thu, Feb 24, 2011 at 11:07 PM, Simon Pepping <spepp...@leverkruid.eu
> >> > >wrote:
> >> > >
> >> > > > As before, I will generally not fix findbugs errors or warnings in
> >> > > > contributions by other people. I will fix findbugs errors or warnings
> >> > > > in code that I write, or code changes that I make.
> >> > > >
> >> > > > Note that the use of the findbugs code analysis tool is not a policy
> >> > > > of the FOP project, and that consequently FOP committers are not
> >> > > > bound to use findbugs and fix its errors or warnings.
> >> > > >
> >> > > > Simon
> >> > > >
> >> > > > On Thu, Feb 24, 2011 at 04:57:57PM -0800, Glenn Adams wrote:
> >> > > > > I think the existing exclusions should be left in trunk, and that 
> >> > > > > no
> >> > new
> >> > > > > ones should be permitted in (or should be fixed immediately). If 
> >> > > > > you
> >> > do
> >> > > > as
> >> > > > > you suggest below, then the list of findbugs errors will just
> >> > continue to
> >> > > > > grow because nobody will pay attention to them.
> >> > > > >
> >> > > > > We are at a known, stable point, we do have some exclusions that we
> >> > know
> >> > > > > need fixing, and we can do that as time permits; but let's keep it
> >> > that
> >> > > > way
> >> > > > > and not backpedal by allowing in new ones.
> >> > > > >
> >> > > > > G.
> >> > > > >
> >> > > > > On Thu, Feb 24, 2011 at 8:40 AM, Andreas Delmelle <
> >> > > > > andreas.delme...@telenet.be> wrote:
> >> > > > >
> >> > > > > >
> >> > > > > > No response to any of the posts in particular, just a general
> >> > > > > > thought/proposal.
> >> > > > > >
> >> > > > > > I can appreciate that the ComplexScripts branch requires a clean 
> >> > > > > > FB
> >> > > > report
> >> > > > > > so that Glenn is not continuously sent on a wild goose chase.
> >> > > > > > However, personally (and Vincent seems to agree), I do not favor
> >> > > > 'blind'
> >> > > > > > exclusions just to make the warnings go away. Following the same
> >> > > > reasoning,
> >> > > > > > we could define thousands of CheckStyle suppressions, instead of
> >> > > > encouraging
> >> > > > > > people to do it correctly.
> >> > > > > >
> >> > > > > > I do not have a problem with looking into those issues, if no one
> >> > else
> >> > > > has
> >> > > > > > the time and/or motivation, although that will not always happen
> >> > > > > > _immediately_.
> >> > > > > >
> >> > > > > > The general idea is good, but I am wondering, given the
> >> > circumstances,
> >> > > > if
> >> > > > > > we had not better invert the approach: keep the warnings alive in
> >> > > > trunk, and
> >> > > > > > add exclusions for them only in the branch.
> >> > > > > > That way, devs who are not involved in the branch but do use FB,
> >> > will
> >> > > > be
> >> > > > > > constantly reminded that those issues should be looked into. For
> >> > the
> >> > > > > > maintainer(s) of the branch, if the exclusion is properly
> >> > commented, it
> >> > > > can
> >> > > > Jeremias Maerki
> >
> >




Jeremias Maerki

Reply via email to