You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by sp...@apache.org on 2011/02/18 09:18:05 UTC
svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml
src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
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
==============================================================================
--- xmlgraphics/fop/trunk/findbugs-exclude.xml (original)
+++ xmlgraphics/fop/trunk/findbugs-exclude.xml Fri Feb 18 08:18:04 2011
@@ -5019,4 +5019,51 @@
<Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
</Match>
<!-- /Automatically generated list of exclusions -->
+ <!-- Automatically generated list of exclusions on 18 February 2011 -->
+ <Match>
+ <Class name="org.apache.fop.afp.goca.GraphicsSetProcessColor"/>
+ <Method name="writeToStream"/>
+ <Bug pattern="OS_OPEN_STREAM"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.pdf.PDFFactory"/>
+ <Method name="makeSeparationColorSpace"/>
+ <Bug pattern="DM_FP_NUMBER_CTOR"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.pdf.PDFFactory"/>
+ <Method name="toColorVector"/>
+ <Bug pattern="DM_FP_NUMBER_CTOR"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.fo.expr.PropertyTokenizer"/>
+ <Field name="recognizeOperator"/>
+ <Bug pattern="URF_UNREAD_FIELD"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.layoutmgr.BlockLayoutManager"/>
+ <Method name="getNextChildElements"/>
+ <Bug pattern="BC_UNCONFIRMED_CAST"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.util.ColorWithFallback"/>
+ <!-- Listing the method 'equals' does not work -->
+ <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.util.ColorWithFallback"/>
+ <Method name="getAlternativeColors"/>
+ <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.fo.expr.NamedColorFunction"/>
+ <Method name="eval"/>
+ <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
+ </Match>
+ <Match>
+ <Class name="org.apache.fop.util.ColorUtil"/>
+ <Method name="parseAsFopRgbNamedColor"/>
+ <Bug pattern="REC_CATCH_EXCEPTION"/>
+ </Match>
+ <!-- /Automatically generated list of exclusions on 18 February 2011 -->
</FindBugsFilter>
\ No newline at end of file
Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java?rev=1071912&r1=1071911&r2=1071912&view=diff
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Fri Feb 18 08:18:04 2011
@@ -60,7 +60,8 @@ public class PDFImageHandlerSVG implemen
private static Log log = LogFactory.getLog(PDFImageHandlerSVG.class);
/** {@inheritDoc} */
- public void handleImage(RenderingContext context, Image image, Rectangle pos)
+ public void handleImage(RenderingContext context, // CSOK: MethodLength
+ Image image, Rectangle pos)
throws IOException {
PDFRenderingContext pdfContext = (PDFRenderingContext)context;
PDFContentGenerator generator = pdfContext.getGenerator();
---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
Posted by Glenn Adams <gl...@skynav.com>.
I support enabling assertions by default on junit tests. More testing is a
good thing.
G.
On Tue, Feb 22, 2011 at 3:40 PM, Andreas Delmelle <
andreas.delmelle@telenet.be> wrote:
> On 22 Feb 2011, at 19:25, Andreas Delmelle wrote:
>
> > One way to enable assertions I found so far, and that seems to work:
> > - use fork="true" on the <junit> target
> > - insert: <assertions><enabled /></assertions> in that target
> >
> > That seems to yield the expected behavior at first glance. Our junit
> targets are not forked, however. Not sure if that is preferable...
>
> FWIW, having configured this on the junit-layout-standard target, yields
> one AssertionError
>
> [junit] Testcase:
> block-container_reference-orientation_bug36391.xml(org.apache.fop.layoutengine.LayoutEngineTestSuite$1):
> Caused an ERROR
> [junit] null
> [junit] java.lang.AssertionError
> [junit] at
> org.apache.fop.layoutmgr.BreakingAlgorithm.getLineWidth(BreakingAlgorithm.java:1394)
>
> ...
>
>
> For the remainder, all layout tests pass.
>
>
> Regards,
>
> Andreas
> ---
>
>
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 19:25, Andreas Delmelle wrote:
> One way to enable assertions I found so far, and that seems to work:
> - use fork="true" on the <junit> target
> - insert: <assertions><enabled /></assertions> in that target
>
> That seems to yield the expected behavior at first glance. Our junit targets are not forked, however. Not sure if that is preferable...
FWIW, having configured this on the junit-layout-standard target, yields one AssertionError
[junit] Testcase: block-container_reference-orientation_bug36391.xml(org.apache.fop.layoutengine.LayoutEngineTestSuite$1):
Caused an ERROR
[junit] null
[junit] java.lang.AssertionError
[junit] at org.apache.fop.layoutmgr.BreakingAlgorithm.getLineWidth(BreakingAlgorithm.java:1394)
...
For the remainder, all layout tests pass.
Regards,
Andreas
---
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 19:04, Andreas Delmelle wrote:
> On 22 Feb 2011, at 08:50, Simon Pepping wrote:
>> It would be nice if assertions were checked during the junit tests.
>> However, when I set: <junit ... jvm="java -ea"/>, I get
>> junit.framework.AssertionFailedError errors. How can this be done?
>
> Sounds like a good idea.
> IIC, simply using "-ea" would enable internal assertions for JUnit's classes as well.
> You probably want something like "-ea:org.apache.fop..." to make sure only assertions in FOP's code are taken into account.
Tried that and failed.. :(
One way to enable assertions I found so far, and that seems to work:
- use fork="true" on the <junit> target
- insert: <assertions><enabled /></assertions> in that target
That seems to yield the expected behavior at first glance. Our junit targets are not forked, however. Not sure if that is preferable...
Regards,
Andreas
---
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 08:50, Simon Pepping wrote:
> On Mon, Feb 21, 2011 at 08:28:33PM +0100, Andreas Delmelle wrote:
>> I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this.
>
> It would be nice if assertions were checked during the junit tests.
> However, when I set: <junit ... jvm="java -ea"/>, I get
> junit.framework.AssertionFailedError errors. How can this be done?
Sounds like a good idea.
IIC, simply using "-ea" would enable internal assertions for JUnit's classes as well.
You probably want something like "-ea:org.apache.fop..." to make sure only assertions in FOP's code are taken into account.
Regards,
Andreas
---
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Mon, Feb 21, 2011 at 08:28:33PM +0100, Andreas Delmelle wrote:
> I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this.
It would be nice if assertions were checked during the junit tests.
However, when I set: <junit ... jvm="java -ea"/>, I get
junit.framework.AssertionFailedError errors. How can this be done?
Simon
Re: Solving FindBugs issue
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 08:34, Simon Pepping wrote:
> When I build the project or part of it with Eclipse, and run findbugs
> afterwards (with ant), I get a number of errors. Now I always make a
> clean compile before running findbugs. I do not understand why Eclipse
> builds would create findbugs errors where a clean ant build does not.
> It makes findbugs seem fickle.
I seem to have the reverse issue here: the Ant target yielded warnings that the FB plugin for IntelliJ IDEA does not complain about (same FB version, same exclusion file)... Maybe the developers of those plugins were not using FindBugs themselves. ;-)
Regards,
Andreas
---
Re: junit tests in nightly builds [was: Re: Solving FindBugs issue]
Posted by Glenn Adams <gl...@skynav.com>.
BTW, if someone would bother to nominate me as a committer, I would be happy
to do this work directly. I don't like depending on the other committers to
make progress in this area any more than you like having to address it.
G.
On Wed, Feb 23, 2011 at 1:15 AM, Glenn Adams <gl...@skynav.com> wrote:
> OK, understand on the junit headless issue. For checkstyle/findbugs it
> would be useful to fail the nightly build if they do not pass. I will
> investigate the necessary changes to enable this option, which I hope can be
> adopted.
>
> G.
>
>
> On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
>
>> On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
>> > I notice also that the nightly build target does not run all the junit
>> > tests. It would be better if it run all of them plus checkstyle and
>> > findbugs.
>>
>> Many junit tests require a display. Nightly builds are run in a
>> headless configuration, hence I had to disable many junit tests. At
>> nightly builds there is no one to check checkstyle and findbugs errors
>> and warnings; therefore there is no point in running them.
>>
>> Simon
>>
>
>
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
Vincent, you can be sure that I don't like pushing any more than you like
hearing it. At the same time, I am dissatisfied with the delays in taking
action, and I believe it is an impediment to my ability to contribute. And
it is becoming an impediment to my willingness to contribute as well. I have
heard it expressed here a few times that committer status is viewed as a
form of reward. I think that is the wrong way to look at it. A better way is
to view it as a calculated risk (on the part of the PMC) in exchange for
greater positive work and progress in the near term. It would be better to
have a process that can quickly assign committer status on a candidate or
temporary basis and then make a subsequent decision on making it permanent.
This creates a sort of probationary status during which the candidate
committer's work is under perhaps greater scrutiny and can be more easily
revoked. A problem with the current approach is that it tends to squelch
initial levels of high interest, when the most benefits can be accrued from
contributions. I'm not saying this just for my own benefit either; I believe
the FOP project needs to find ways to accelerate its work, and that means
recruiting committers more quickly than more slowly. It would be useful to
know the typical period of granting committer status in other ASF projects,
and correlating this period to the rate of development in the projects.
G.
On Thu, Mar 3, 2011 at 4:33 AM, Vincent Hennebert <vh...@gmail.com>wrote:
> On 02/03/11 22:44, Glenn Adams wrote:
> > My apologies for mixing the FB discussion and the commitership issue,
> which
> > are indeed separate.
> <snip/>
> > Now, regarding commitership, I must say that if I were already a
> committer,
> > I would probably have already fixed all the current FB exclusions or
> > documented the chaff as acceptable. However, having to go through other
> > committers to fix up all these issues is simply too much pain, partly
> > because it places more delays into the process and makes it harder to
> > implement such fixes in the face of a moving target. Having spent at
> least 8
> > months contributing to FOP, I find it disconcerting that (1) I am still
> > waiting feedback about committer status, (2) I have not even been
> > acknowledged as a contributor (see
> > http://xmlgraphics.apache.org/fop/team.html). Frankly, the process
> appears
> > to be broken.
>
> In general, the more people have the impression that they are being
> forced to make a decision, the more resistant to it they are.
>
> Vincent
>
Re: FindBugs exclusion policy proposal
Posted by Vincent Hennebert <vh...@gmail.com>.
On 02/03/11 22:44, Glenn Adams wrote:
> My apologies for mixing the FB discussion and the commitership issue, which
> are indeed separate.
<snip/>
> Now, regarding commitership, I must say that if I were already a committer,
> I would probably have already fixed all the current FB exclusions or
> documented the chaff as acceptable. However, having to go through other
> committers to fix up all these issues is simply too much pain, partly
> because it places more delays into the process and makes it harder to
> implement such fixes in the face of a moving target. Having spent at least 8
> months contributing to FOP, I find it disconcerting that (1) I am still
> waiting feedback about committer status, (2) I have not even been
> acknowledged as a contributor (see
> http://xmlgraphics.apache.org/fop/team.html). Frankly, the process appears
> to be broken.
In general, the more people have the impression that they are being
forced to make a decision, the more resistant to it they are.
Vincent
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
thanks
On Fri, Mar 4, 2011 at 2:27 AM, Chris Bowditch
<bo...@hotmail.com>wrote:
> On 03/03/2011 17:30, Glenn Adams wrote:
>
> Hi Glenn,
>
> On Thu, Mar 3, 2011 at 5:58 AM, Chris Bowditch <
>> bowditch_chris@hotmail.com <ma...@hotmail.com>> wrote:
>>
>> Please send me a short bio and I will add you to the team page.
>>
>>
> Chris, per your request, an abridged bio follows:
>>
>> "Dr. Glenn Adams is an independent consultant with 45+ years of
>> professional software design and development experience in areas ranging
>> from embedded, real-time systems, including military, marine and consumer
>> electronics process control and digital signal processing applications, to
>> advanced typographic and multilingual text processing systems. He is former
>> technical director of the Unicode Consortium, and a co-author of the Unicode
>> Standard. He has 25+ years experience in developing international standards,
>> serving on ISO JTC1/SC2 (Coded Character Sets) and JTC1/SC18/WG8 (Document
>> Processing) committees, and was actively involved in the creation of ISO
>> 8879 (SGML), ISO/IEC 9541 (Font Information Interchange), ISO/IEC 10179
>> (DSSSL), ISO/IEC 10180 (SPDL), ISO/IEC 10646 (Universal Coded Character
>> Set), and, within the W3C, on XSL-FO 1.0. In addition, Dr. Adams is an
>> active member of the Amateur Radio community, holding call sign N0GNR."
>>
>> Feel free to abridge further if this is too long.
>>
> It is a little on the long side compared to the others, but I couldn't see
> much opportunity for abridging further as most of it is relevant
> information. I did decide to remove the bit about the Amateur Radio since
> that isn't relevant to your role in FOP. I hope that is ok?
>
> Modifed team page committed here:
> http://permalink.gmane.org/gmane.text.xml.fop.cvs/9815
>
> This will show up when the website is next published.
>
>
>
>> You may also wish to consider reviewing the "Active Committers" list, as
>> there are a few names there for which I have seen no activity for some time.
>>
>
> I noticed a couple of active contributors that I have not seen or heard
> from in some time and decided to delete 1 whilst I was there.
>
>>
>> G.
>>
>>
> Thanks,
>
> Chris
>
>
Re: FindBugs exclusion policy proposal
Posted by Chris Bowditch <bo...@hotmail.com>.
On 03/03/2011 17:30, Glenn Adams wrote:
Hi Glenn,
> On Thu, Mar 3, 2011 at 5:58 AM, Chris Bowditch
> <bowditch_chris@hotmail.com <ma...@hotmail.com>> wrote:
>
> Please send me a short bio and I will add you to the team page.
>
> Chris, per your request, an abridged bio follows:
>
> "Dr. Glenn Adams is an independent consultant with 45+ years of
> professional software design and development experience in areas
> ranging from embedded, real-time systems, including military, marine
> and consumer electronics process control and digital signal processing
> applications, to advanced typographic and multilingual text processing
> systems. He is former technical director of the Unicode Consortium,
> and a co-author of the Unicode Standard. He has 25+ years experience
> in developing international standards, serving on ISO JTC1/SC2 (Coded
> Character Sets) and JTC1/SC18/WG8 (Document Processing) committees,
> and was actively involved in the creation of ISO 8879 (SGML), ISO/IEC
> 9541 (Font Information Interchange), ISO/IEC 10179 (DSSSL), ISO/IEC
> 10180 (SPDL), ISO/IEC 10646 (Universal Coded Character Set), and,
> within the W3C, on XSL-FO 1.0. In addition, Dr. Adams is an active
> member of the Amateur Radio community, holding call sign N0GNR."
>
> Feel free to abridge further if this is too long.
It is a little on the long side compared to the others, but I couldn't
see much opportunity for abridging further as most of it is relevant
information. I did decide to remove the bit about the Amateur Radio
since that isn't relevant to your role in FOP. I hope that is ok?
Modifed team page committed here:
http://permalink.gmane.org/gmane.text.xml.fop.cvs/9815
This will show up when the website is next published.
>
> You may also wish to consider reviewing the "Active Committers" list,
> as there are a few names there for which I have seen no activity for
> some time.
I noticed a couple of active contributors that I have not seen or heard
from in some time and decided to delete 1 whilst I was there.
>
> G.
>
Thanks,
Chris
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
On Thu, Mar 3, 2011 at 5:58 AM, Chris Bowditch
<bo...@hotmail.com>wrote:
> Please send me a short bio and I will add you to the team page.
>
Chris, per your request, an abridged bio follows:
"Dr. Glenn Adams is an independent consultant with 45+ years of professional
software design and development experience in areas ranging from embedded,
real-time systems, including military, marine and consumer electronics
process control and digital signal processing applications, to advanced
typographic and multilingual text processing systems. He is former technical
director of the Unicode Consortium, and a co-author of the Unicode Standard.
He has 25+ years experience in developing international standards, serving
on ISO JTC1/SC2 (Coded Character Sets) and JTC1/SC18/WG8 (Document
Processing) committees, and was actively involved in the creation of ISO
8879 (SGML), ISO/IEC 9541 (Font Information Interchange), ISO/IEC 10179
(DSSSL), ISO/IEC 10180 (SPDL), ISO/IEC 10646 (Universal Coded Character
Set), and, within the W3C, on XSL-FO 1.0. In addition, Dr. Adams is an
active member of the Amateur Radio community, holding call sign N0GNR."
Feel free to abridge further if this is too long.
You may also wish to consider reviewing the "Active Committers" list, as
there are a few names there for which I have seen no activity for some time.
G.
Re: FindBugs exclusion policy proposal
Posted by Chris Bowditch <bo...@hotmail.com>.
On 02/03/2011 22:44, Glenn Adams wrote:
Hi Glenn,
<snip/>
>
> Now, regarding commitership, I must say that if I were already a
> committer, I would probably have already fixed all the current FB
> exclusions or documented the chaff as acceptable. However, having to
> go through other committers to fix up all these issues is simply too
> much pain, partly because it places more delays into the process and
> makes it harder to implement such fixes in the face of a moving
> target. Having spent at least 8 months contributing to FOP, I find it
> disconcerting that (1) I am still waiting feedback about committer
> status, (2) I have not even been acknowledged as a contributor (see
> http://xmlgraphics.apache.org/fop/team.html). Frankly, the process
> appears to be broken.
You do indeed have a point. I don't think I would go as far as saying
the process is broken, just a bit slow. I do agree that you at least
deserve to be classed as an official Contributor. Please send me a short
bio and I will add you to the team page.
As for committership criteria, most of the committers have a similar
view on requirements. 5-6 useful patches, plus helping out on the user
mailing list. It's true 1 or 2 folks made it without any patches, but
then it did take them a couple of years to reach comitter status and
they were very active on the user list, almost single handedly solving
all issues and helping out with other tasks, such as website content
and/or bug scrubbing.
A time alone does not indicate the value of someones contributions,
which is different for everyone. It is true that you have donated a very
substantial piece of work, but 1 patch alone does not mean you
automatically qualify to be a committer. We want to see a steady flow of
patches over time, plus helping users on the mailing list.
>
> G.
>
Thanks,
Chris
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
My apologies for mixing the FB discussion and the commitership issue, which
are indeed separate.
Of course, I have never said FB is a magic wand, etc., and indeed no such
tool is. One makes use of these tools as helpers, and not as determiners. Is
the trouble of using FB regularly worth it? I suppose it depends on one's
perspective. I find it extremely easy to use and easy to check the results.
It serves as merely one more filter on the build and dev process, and does
appear to be able to identify real bugs on occasion. Yes, there is
occasionally some chaff to deal with.
Then again, and perhaps to stretch an analogy, I recall working on the SDI
(strategic defense initiative) in the 80s, and observed an extraordinary
amount of effort and money spent to distinguish the decoy warheads from the
real warheads. Few were willing to argue that because there were 10 - 100
decoys for every real one, then it wasn't worth the trouble to separate
them. Are the "decoy" bugs (false positives) too much to bother with even if
FB finds a few real ones? Personally, I think so.
Now, regarding commitership, I must say that if I were already a committer,
I would probably have already fixed all the current FB exclusions or
documented the chaff as acceptable. However, having to go through other
committers to fix up all these issues is simply too much pain, partly
because it places more delays into the process and makes it harder to
implement such fixes in the face of a moving target. Having spent at least 8
months contributing to FOP, I find it disconcerting that (1) I am still
waiting feedback about committer status, (2) I have not even been
acknowledged as a contributor (see
http://xmlgraphics.apache.org/fop/team.html). Frankly, the process appears
to be broken.
G.
On Wed, Mar 2, 2011 at 3:25 PM, Andreas Delmelle <
andreas.delmelle@telenet.be> wrote:
> <snip />
>
> Just to be clear: my main issue was with maintaining the auto-exclusion
> policy in the longer term. I do not question the past decisions, nor did I
> intend to point fingers (even if only implicitly). If I enervated anyone by
> bringing this up again, my apologies.
> The more general context is avoiding what Glenn referred to as
> 'stagnation'.
>
> I was just concerned about the most current action, because I can see where
> Glenn and Vincent are coming from. I noticed the FB target had been added,
> and was sincerely interested in using it, but if this is how its use will be
> promoted, that interest might very well fade quite soon...
>
> I do not see FB as a magic wand myself, as there are obviously a lot of
> common and less common, in-your-face mistakes, anti-patterns or other types
> of ugliness that do not raise any flag whatsoever, not even a warning, but
> do deserve attention nevertheless. Things that require a human eye and brain
> to detect them.
> I see it more as a learning experience, hoping that in time, one's code
> style will adapt and those dubious patterns are avoided to begin with, hence
> reducing the 'additional effort' required to the time necessary to run the
> build target and check whether the result is still zero...
> It might just make you an even better coder in the long term. Or, maybe you
> think that is just not possible... ;-)
>
> Speaking in a more general context, no matter how many layers of testing
> and automated checks are added, it is always possible to produce the
> proverbial substandard spaghetti. No guarantees there, unfortunately.
> It *will* be reasonably guarded against future regressions and be virtually
> free of functional bugs, though... and have a fairly consistent style, too!
> :->
>
> About the whole committership debate:
> I feel like I should remind people that some --including myself-- got voted
> in without having submitted a single line of code. It's all about
> contributions, in whatever form.
> There have also been people that donated very significant patches, but
> never stayed long enough to be considered.
> Come to think of it, I almost cannot remember when the previous committer
> got voted in. Was that Pascal? Quite some time ago, and I am seeing patches
> in Bugzilla that reveal a few potentials. Just a suggestion. Contact me
> off-list if you do not see them immediately. No name-dropping here. ;-)
> They might be tempted to spend their time elsewhere if their efforts here
> are not 'rewarded' in some way other than just: 'Thanks for the patch!'
>
>
> Regards,
>
> Andreas
> ---
Re: FindBugs exclusion policy proposal
Posted by Andreas Delmelle <an...@telenet.be>.
<snip />
Just to be clear: my main issue was with maintaining the auto-exclusion policy in the longer term. I do not question the past decisions, nor did I intend to point fingers (even if only implicitly). If I enervated anyone by bringing this up again, my apologies.
The more general context is avoiding what Glenn referred to as 'stagnation'.
I was just concerned about the most current action, because I can see where Glenn and Vincent are coming from. I noticed the FB target had been added, and was sincerely interested in using it, but if this is how its use will be promoted, that interest might very well fade quite soon...
I do not see FB as a magic wand myself, as there are obviously a lot of common and less common, in-your-face mistakes, anti-patterns or other types of ugliness that do not raise any flag whatsoever, not even a warning, but do deserve attention nevertheless. Things that require a human eye and brain to detect them.
I see it more as a learning experience, hoping that in time, one's code style will adapt and those dubious patterns are avoided to begin with, hence reducing the 'additional effort' required to the time necessary to run the build target and check whether the result is still zero...
It might just make you an even better coder in the long term. Or, maybe you think that is just not possible... ;-)
Speaking in a more general context, no matter how many layers of testing and automated checks are added, it is always possible to produce the proverbial substandard spaghetti. No guarantees there, unfortunately.
It *will* be reasonably guarded against future regressions and be virtually free of functional bugs, though... and have a fairly consistent style, too! :->
About the whole committership debate:
I feel like I should remind people that some --including myself-- got voted in without having submitted a single line of code. It's all about contributions, in whatever form.
There have also been people that donated very significant patches, but never stayed long enough to be considered.
Come to think of it, I almost cannot remember when the previous committer got voted in. Was that Pascal? Quite some time ago, and I am seeing patches in Bugzilla that reveal a few potentials. Just a suggestion. Contact me off-list if you do not see them immediately. No name-dropping here. ;-)
They might be tempted to spend their time elsewhere if their efforts here are not 'rewarded' in some way other than just: 'Thanks for the patch!'
Regards,
Andreas
---
Re: FindBugs exclusion policy proposal
Posted by Benson Margulies <bi...@gmail.com>.
>From the sidelines,
Apache projects are encouraged by the norms of the Foundation to
actively recruit new committers and to recognize community-positive
effort with committer status. Apache projects are, at the same time,
given a very wide latitude by the Foundation in making decisions. In
my experience across several other projects, if someone has made a
significant string of contributions over an extended period of time,
it is reasonable for that someone to expect the PMC to give serious
consideration to committer status and to offer some sort of meaningful
feedback if they choose not to grant it. I suppose that in the extreme
a person who feels unfairly excluded could open communication with the
Foundations' board; that would be a somewhat extreme course, and it
would be a very unusual and extreme situation for the board to dictate
to the PMC, but it is theoretically possible.
As I have something of a conflict of interest here, (I am a Foundation
member and am thus charged with facilitating the Foundation's mission,
but I also pay Glenn to work on FOP), I'll leave my remarks about the
Foundation at that.
As for the situation at hand, I'm not sure that Glenn quite decoded
J.Pietschmann's message, which I read as explaining the current
situation in terms of a sleepy PMC rather than an intentionally
exclusive one. Or perhaps he is more interested in the net effect of
the current situation.
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
On Sun, Feb 27, 2011 at 4:18 PM, J.Pietschmann <j3...@yahoo.de> wrote:
> However, saying all of the above, the largest barrier I see to fixing bugs
>> in FOP and improving its quality is the reticence of the clique of FOP
>> committers to accept new committers.
>>
>
> Uh oh. As a PMC member I'm certainly guilty of not following
> contributions closely enough. Nevertheless, rest assured this
> has nothing to do with some elitist attitude but rather with a
> "I hope someone else will deal with this kind of aspects" stance,
> aka a sort of laziness. I do not expect you to show sympathy for this
> now, but in case you get the possibility to recruit someone else, and
> realize the procedures necessary for this, you'll understand it
> better (I might get some fun out of reminding you of this message
> then).
>
I know of no such procedure. There is none published and none communicated.
It appears that an arbitrary, unknown set of criteria hold, with no
mechanism for discovery or feedback.
>From what I can see, it is in fact nothing more than a club of insiders.
If I do have an opportunity to be a PMC member, I will vote for anyone who
volunteers to be a committer on the spot. I would rather bring them inside
sooner than later, and deal with any personality issues after the fact.
Placing an arbitrary, unknown barrier over entry is a sure guarantee for
stagnation, and if you ask me, that is where FOP is at this time: stagnated.
Only Andreas seems to have devoted a new burst of energy to the process of
late, while others appear to have other commitments that prevent them from
contributing except on an occasional basis. In such a case, I can't imagine
why anyone would turn down a volunteer to actively participate, other than
perhaps turf protection. I guess I must threaten someone's turf.
G.
Re: FindBugs exclusion policy proposal
Posted by "J.Pietschmann" <j3...@yahoo.de>.
On 27.02.2011 02:46, Glenn Adams wrote:
> My expectation (not hope) was that once we eliminated the pre-existing
> findbugs, that new commits would not add new issues. Otherwise, it is a
> pointless exercise. My conclusion is that since findbugs does indeed
> identify some real and potential bugs, that it is worth using and that a
> zero findbugs policy should apply.
Well, while findbugs indeed finds real issues and improves
maintainability of the code, I don't think I'd have enough
time to commit myself to examine each and every findbugs
complaint carefully, rather then just plaster it over with
an exception, and I think it's the same with Jeremias.
This doesn't mean "we throw it out." As of now, anybody is free
to run the findbugs target and fix issues which come up. Installing
a zero findbugs issues policy is a rather drastic step, which will,
believe it or not, raise barriers for contributions, divert
resources and possibly have some other undesirable effects, like
name calling on public lists. Unless we all are convinced we have
the resources to lead by example and that the positive effects of
a "zero findbugs issues" policy outweigh any possible drawbacks,
running findbugs should be voluntary.
> However, saying all of the above, the largest barrier I see to fixing bugs
> in FOP and improving its quality is the reticence of the clique of FOP
> committers to accept new committers.
Uh oh. As a PMC member I'm certainly guilty of not following
contributions closely enough. Nevertheless, rest assured this
has nothing to do with some elitist attitude but rather with a
"I hope someone else will deal with this kind of aspects" stance,
aka a sort of laziness. I do not expect you to show sympathy for this
now, but in case you get the possibility to recruit someone else, and
realize the procedures necessary for this, you'll understand it
better (I might get some fun out of reminding you of this message
then).
HTH
J.Pietschmann
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
On Sat, Feb 26, 2011 at 2:14 PM, J.Pietschmann <j3...@yahoo.de> wrote:
> On 26.02.2011 15:05, Glenn Adams wrote:
>
>>
>>> Establishing a zero-FindBugs policy at build level: absolutely not.
>>>
>>
>>
>> So you offer no rationale or reason for such an opinion? Or your only
>> reason
>> for this is that FindBugs is not perfect? Or because you find it too
>> troublesome to type "ant findbugs" and look at the results?
>>
>
>
> I haven't looked at your patch which fixed all the findbugs issues
> in FOP. Do you have the numbers of false positives and some
> estimates about the impact the real bugs would have had? If the
> numbers prove that findbugs systematically adds value, it might
> be worth changing policy.
>
I didn't actually fix the findbugs issues that predated my addition of the
findbugs-exclude file. Simon created a script that automatically added the
exclusions in order to zero out the findbugs error count. The hope (I would
not say expectation) was that as time permitted, folks would take the
opportunity to go back and review those pre-existing bugs and either make
fixes or add "blessed" exclusions, meaning adding a comment at least in the
exclusion file indicating the fact that review had occurred and a decision
had been taken to make the exclusion for a stated reason.
My experience so far with findbugs is that a reasonable percentage of the
issues it reports are indeed real or potential bugs, and that a relatively
small percentage (<10%) need to be excluded. Even in many of the latter
cases, the warning was suitable for consideration, e.g., warnings about
returning null or returning an empty array.
My expectation (not hope) was that once we eliminated the pre-existing
findbugs, that new commits would not add new issues. Otherwise, it is a
pointless exercise. My conclusion is that since findbugs does indeed
identify some real and potential bugs, that it is worth using and that a
zero findbugs policy should apply.
I find it hard to believe that professional programmers would think
differently.
Of course, no tool of this sort is perfect or even near so, but to simply
throw it out because it takes a few minutes to check and fix new issues
seems counterproductive to any stated goals of improving quality, not just
code quality but functional quality.
I noticed in a recent status report on FOP that it was felt worth mentioning
the work to improve quality. Someone apparently thought that worth pointing
out to others in Apache and elsewhere. Why we would not continue that policy
in using findbugs as well seems rather shocking to me.
In the numerous large software development projects I have led over the past
40 years, I have consistently used whatever tools were available, no matter
how imperfect, in the hope that they will identify a few useful bugs and
give some added confidence that a reasonable amount of due diligence was
applied in finding and fixing bugs.
However, saying all of the above, the largest barrier I see to fixing bugs
in FOP and improving its quality is the reticence of the clique of FOP
committers to accept new committers. Rather than bring in new blood and new
energy, folks seem to prefer to complain about the length of the bug list
and wring their hands and do nothing or little to improve the situation.
I have offered on numerous occasions to contribute actively to FOP, but I
cannot do so because I am not a committer, and having to go through
committers to make improvements is just too troublesome. My energy to
improve FOP has diminished considerably as a result.
G.
Re: FindBugs exclusion policy proposal
Posted by "J.Pietschmann" <j3...@yahoo.de>.
On 26.02.2011 15:05, Glenn Adams wrote:
>>
>> Establishing a zero-FindBugs policy at build level: absolutely not.
>
>
> So you offer no rationale or reason for such an opinion? Or your only reason
> for this is that FindBugs is not perfect? Or because you find it too
> troublesome to type "ant findbugs" and look at the results?
From my point of view, it's a matter of diminishing returns.
The basic checkstyle rule set does a lot for improving
maintainability, and complaints are easily fixed.
While findbugs (as well as more sophisticated checkstyle rule
sets) uncovers additional problems, it also produces quite a
few false positives. This causes some significant effort spent
on deciding whether a findbugs complaint is really a bug which
must be fixed, diverting our scarce resources from possibly more
pressing problems, like implementing features many users are
already waiting for years.
Last time I checked, findbugs had around 50% false positives for
my code base, and it had missed *all* important bugs which had
burnt me in the months before. This numbers would not be good
enough to install findbugs usage as a policy. And there were cases
where fixing a findbugs complaint got tricky, meaning pushing out
a feature would be delayed for *all* users because someone,
somewhere *might* see a malfunction.
I haven't looked at your patch which fixed all the findbugs issues
in FOP. Do you have the numbers of false positives and some
estimates about the impact the real bugs would have had? If the
numbers prove that findbugs systematically adds value, it might
be worth changing policy.
For some more info, and some good laugh, you might want to read
the following article:
http://cacm.acm.org/magazines/2010/2/69354-a-few-billion-lines-of-code-later/fulltext
Regards
J.Pietschmann
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
>
> Establishing a zero-FindBugs policy at build level: absolutely not.
So you offer no rationale or reason for such an opinion? Or your only reason
for this is that FindBugs is not perfect? Or because you find it too
troublesome to type "ant findbugs" and look at the results?
Shocking.
G.
On Sat, Feb 26, 2011 at 3:26 AM, Jeremias Maerki <de...@jeremias-maerki.ch>wrote:
> 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 <de...@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 <
> dev@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 <
> spepping@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.delmelle@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
>
>
Re: FindBugs exclusion policy proposal
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
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 <de...@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 <de...@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 <spepping@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.delmelle@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
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
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 <de...@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 <de...@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 <spepping@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.delmelle@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
>
>
Re: FindBugs exclusion policy proposal
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
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 <de...@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 <spepping@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.delmelle@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
> > > > > > serve as an indication that the warning originated in trunk and has
> > > > nothing
> > > > > > to do with their changes. Should a genuine bug result from it, and
> > it
> > > > turns
> > > > > > out to hamper the development on the branch, it can then be raised
> > as a
> > > > > > priority issue on this list.
> > > > > >
> > > > > > Ultimately, it is still a worthwhile goal to eliminate all of the
> > > > warnings,
> > > > > > but we also have to be realistic enough to admit that that will not
> > > > happen
> > > > > > overnight.
> > > > > >
> > > > > >
> > > > > > WDYT?
> > > > > >
> > > > > > Regards,
> > > > > >
> > > > > > Andreas
> > > > > > ---
> > > > > >
> > > > > >
> > > >
> >
> >
> >
> >
> > Jeremias Maerki
> >
> >
Jeremias Maerki
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
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 <de...@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 <spepping@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.delmelle@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
> > > > > serve as an indication that the warning originated in trunk and has
> > > nothing
> > > > > to do with their changes. Should a genuine bug result from it, and
> it
> > > turns
> > > > > out to hamper the development on the branch, it can then be raised
> as a
> > > > > priority issue on this list.
> > > > >
> > > > > Ultimately, it is still a worthwhile goal to eliminate all of the
> > > warnings,
> > > > > but we also have to be realistic enough to admit that that will not
> > > happen
> > > > > overnight.
> > > > >
> > > > >
> > > > > WDYT?
> > > > >
> > > > > Regards,
> > > > >
> > > > > Andreas
> > > > > ---
> > > > >
> > > > >
> > >
>
>
>
>
> Jeremias Maerki
>
>
Re: FindBugs exclusion policy proposal
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
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 <sp...@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.delmelle@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
> > > > serve as an indication that the warning originated in trunk and has
> > nothing
> > > > to do with their changes. Should a genuine bug result from it, and it
> > turns
> > > > out to hamper the development on the branch, it can then be raised as a
> > > > priority issue on this list.
> > > >
> > > > Ultimately, it is still a worthwhile goal to eliminate all of the
> > warnings,
> > > > but we also have to be realistic enough to admit that that will not
> > happen
> > > > overnight.
> > > >
> > > >
> > > > WDYT?
> > > >
> > > > Regards,
> > > >
> > > > Andreas
> > > > ---
> > > >
> > > >
> >
Jeremias Maerki
Re: FindBugs exclusion policy proposal
Posted by Glenn Adams <gl...@skynav.com>.
Well then, let's make it a policy. What's preventing that?
On Thu, Feb 24, 2011 at 11:07 PM, Simon Pepping <sp...@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.delmelle@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
> > > serve as an indication that the warning originated in trunk and has
> nothing
> > > to do with their changes. Should a genuine bug result from it, and it
> turns
> > > out to hamper the development on the branch, it can then be raised as a
> > > priority issue on this list.
> > >
> > > Ultimately, it is still a worthwhile goal to eliminate all of the
> warnings,
> > > but we also have to be realistic enough to admit that that will not
> happen
> > > overnight.
> > >
> > >
> > > WDYT?
> > >
> > > Regards,
> > >
> > > Andreas
> > > ---
> > >
> > >
>
Re: FindBugs exclusion policy proposal
Posted by Simon Pepping <sp...@leverkruid.eu>.
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.delmelle@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
> > serve as an indication that the warning originated in trunk and has nothing
> > to do with their changes. Should a genuine bug result from it, and it turns
> > out to hamper the development on the branch, it can then be raised as a
> > priority issue on this list.
> >
> > Ultimately, it is still a worthwhile goal to eliminate all of the warnings,
> > but we also have to be realistic enough to admit that that will not happen
> > overnight.
> >
> >
> > WDYT?
> >
> > Regards,
> >
> > Andreas
> > ---
> >
> >
Re: FindBugs exclusion policy proposal (was: Re: junit tests in
nightly builds etc.)
Posted by Glenn Adams <gl...@skynav.com>.
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.delmelle@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
> serve as an indication that the warning originated in trunk and has nothing
> to do with their changes. Should a genuine bug result from it, and it turns
> out to hamper the development on the branch, it can then be raised as a
> priority issue on this list.
>
> Ultimately, it is still a worthwhile goal to eliminate all of the warnings,
> but we also have to be realistic enough to admit that that will not happen
> overnight.
>
>
> WDYT?
>
> Regards,
>
> Andreas
> ---
>
>
FindBugs exclusion policy proposal (was: Re: junit tests in nightly builds etc.)
Posted by Andreas Delmelle <an...@telenet.be>.
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 serve as an indication that the warning originated in trunk and has nothing to do with their changes. Should a genuine bug result from it, and it turns out to hamper the development on the branch, it can then be raised as a priority issue on this list.
Ultimately, it is still a worthwhile goal to eliminate all of the warnings, but we also have to be realistic enough to admit that that will not happen overnight.
WDYT?
Regards,
Andreas
---
Re: junit tests in nightly builds
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Wed, Feb 23, 2011 at 01:10:10PM -0700, Glenn Adams wrote:
> Right now the nightly build is our CI process.
That is a development-centric point of view. The nightly build is
definitely not a CI process. It is a service to the users, which must
not be interrupted by development concerns. If a reliable application
can be built, it must be built and delivered.
Simon
Re: junit tests in nightly builds
Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Well, not quite: http://vmgump.apache.org/gump/public/xml-fop/
Basics work but it needs some love apparently.
On 23.02.2011 21:10:10 Glenn Adams wrote:
> Right now the nightly build is our CI process.
>
> On Wednesday, February 23, 2011, Vincent Hennebert <vh...@gmail.com> wrote:
> > On 23/02/11 14:42, Glenn Adams wrote:
> >> I guess we disagree, since I believe application quality and code quality
> >> are related. And, further, I believe findbugs at least can identify real,
> >> functional bugs (as opposed to checkstyle).
> >
> > While I agree with the above, Im with Simon on this. Tests should be
> > done within a continuous integration tool; Nightly builds serve
> > a different purpose.
> >
> > I increasingly feel the need to set up continuous integration for the
> > FOP project. The ASF provides several CI environments (Hudson, among
> > others), at some point in the future Im going to try them out and set
> > up something. Hopefully sooner rather than later.
> >
> >
> > Vincent
> >
> >
> >> G.
> >>
> >> On Wed, Feb 23, 2011 at 2:01 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
> >>
> >>> On Wed, Feb 23, 2011 at 01:15:34AM -0700, Glenn Adams wrote:
> >>>> OK, understand on the junit headless issue. For checkstyle/findbugs it
> >>> would
> >>>> be useful to fail the nightly build if they do not pass. I will
> >>> investigate
> >>>> the necessary changes to enable this option, which I hope can be adopted.
> >>>
> >>> I would not agree. Nightly builds are a courtesy to the user. It would
> >>> be good if we could guarantee that the builds pass the junit tests.
> >>> But it is not relevant to the user whether they pass checkstyle and
> >>> findbugs rules. These tests address the issue of code quality, not of
> >>> application quality.
> >>>
> >>> Simon
> >>>
> >>>> On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <spepping@leverkruid.eu
> >>>> wrote:
> >>>>
> >>>>> On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
> >>>>>> I notice also that the nightly build target does not run all the
> >>> junit
> >>>>>> tests. It would be better if it run all of them plus checkstyle and
> >>>>>> findbugs.
> >>>>>
> >>>>> Many junit tests require a display. Nightly builds are run in a
> >>>>> headless configuration, hence I had to disable many junit tests. At
> >>>>> nightly builds there is no one to check checkstyle and findbugs errors
> >>>>> and warnings; therefore there is no point in running them.
> >>>>>
> >>>>> Simon
> >>>>>
> >>>
> >>
> >
Jeremias Maerki
Re: junit tests in nightly builds
Posted by Glenn Adams <gl...@skynav.com>.
Right now the nightly build is our CI process.
On Wednesday, February 23, 2011, Vincent Hennebert <vh...@gmail.com> wrote:
> On 23/02/11 14:42, Glenn Adams wrote:
>> I guess we disagree, since I believe application quality and code quality
>> are related. And, further, I believe findbugs at least can identify real,
>> functional bugs (as opposed to checkstyle).
>
> While I agree with the above, I’m with Simon on this. Tests should be
> done within a continuous integration tool; Nightly builds serve
> a different purpose.
>
> I increasingly feel the need to set up continuous integration for the
> FOP project. The ASF provides several CI environments (Hudson, among
> others), at some point in the future I’m going to try them out and set
> up something. Hopefully sooner rather than later.
>
>
> Vincent
>
>
>> G.
>>
>> On Wed, Feb 23, 2011 at 2:01 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
>>
>>> On Wed, Feb 23, 2011 at 01:15:34AM -0700, Glenn Adams wrote:
>>>> OK, understand on the junit headless issue. For checkstyle/findbugs it
>>> would
>>>> be useful to fail the nightly build if they do not pass. I will
>>> investigate
>>>> the necessary changes to enable this option, which I hope can be adopted.
>>>
>>> I would not agree. Nightly builds are a courtesy to the user. It would
>>> be good if we could guarantee that the builds pass the junit tests.
>>> But it is not relevant to the user whether they pass checkstyle and
>>> findbugs rules. These tests address the issue of code quality, not of
>>> application quality.
>>>
>>> Simon
>>>
>>>> On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <spepping@leverkruid.eu
>>>> wrote:
>>>>
>>>>> On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
>>>>>> I notice also that the nightly build target does not run all the
>>> junit
>>>>>> tests. It would be better if it run all of them plus checkstyle and
>>>>>> findbugs.
>>>>>
>>>>> Many junit tests require a display. Nightly builds are run in a
>>>>> headless configuration, hence I had to disable many junit tests. At
>>>>> nightly builds there is no one to check checkstyle and findbugs errors
>>>>> and warnings; therefore there is no point in running them.
>>>>>
>>>>> Simon
>>>>>
>>>
>>
>
Re: junit tests in nightly builds
Posted by Vincent Hennebert <vh...@gmail.com>.
On 23/02/11 14:42, Glenn Adams wrote:
> I guess we disagree, since I believe application quality and code quality
> are related. And, further, I believe findbugs at least can identify real,
> functional bugs (as opposed to checkstyle).
While I agree with the above, I’m with Simon on this. Tests should be
done within a continuous integration tool; Nightly builds serve
a different purpose.
I increasingly feel the need to set up continuous integration for the
FOP project. The ASF provides several CI environments (Hudson, among
others), at some point in the future I’m going to try them out and set
up something. Hopefully sooner rather than later.
Vincent
> G.
>
> On Wed, Feb 23, 2011 at 2:01 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
>
>> On Wed, Feb 23, 2011 at 01:15:34AM -0700, Glenn Adams wrote:
>>> OK, understand on the junit headless issue. For checkstyle/findbugs it
>> would
>>> be useful to fail the nightly build if they do not pass. I will
>> investigate
>>> the necessary changes to enable this option, which I hope can be adopted.
>>
>> I would not agree. Nightly builds are a courtesy to the user. It would
>> be good if we could guarantee that the builds pass the junit tests.
>> But it is not relevant to the user whether they pass checkstyle and
>> findbugs rules. These tests address the issue of code quality, not of
>> application quality.
>>
>> Simon
>>
>>> On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <spepping@leverkruid.eu
>>> wrote:
>>>
>>>> On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
>>>>> I notice also that the nightly build target does not run all the
>> junit
>>>>> tests. It would be better if it run all of them plus checkstyle and
>>>>> findbugs.
>>>>
>>>> Many junit tests require a display. Nightly builds are run in a
>>>> headless configuration, hence I had to disable many junit tests. At
>>>> nightly builds there is no one to check checkstyle and findbugs errors
>>>> and warnings; therefore there is no point in running them.
>>>>
>>>> Simon
>>>>
>>
>
Re: junit tests in nightly builds
Posted by Glenn Adams <gl...@skynav.com>.
I guess we disagree, since I believe application quality and code quality
are related. And, further, I believe findbugs at least can identify real,
functional bugs (as opposed to checkstyle).
G.
On Wed, Feb 23, 2011 at 2:01 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
> On Wed, Feb 23, 2011 at 01:15:34AM -0700, Glenn Adams wrote:
> > OK, understand on the junit headless issue. For checkstyle/findbugs it
> would
> > be useful to fail the nightly build if they do not pass. I will
> investigate
> > the necessary changes to enable this option, which I hope can be adopted.
>
> I would not agree. Nightly builds are a courtesy to the user. It would
> be good if we could guarantee that the builds pass the junit tests.
> But it is not relevant to the user whether they pass checkstyle and
> findbugs rules. These tests address the issue of code quality, not of
> application quality.
>
> Simon
>
> > On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <spepping@leverkruid.eu
> >wrote:
> >
> > > On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
> > > > I notice also that the nightly build target does not run all the
> junit
> > > > tests. It would be better if it run all of them plus checkstyle and
> > > > findbugs.
> > >
> > > Many junit tests require a display. Nightly builds are run in a
> > > headless configuration, hence I had to disable many junit tests. At
> > > nightly builds there is no one to check checkstyle and findbugs errors
> > > and warnings; therefore there is no point in running them.
> > >
> > > Simon
> > >
>
Re: junit tests in nightly builds
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Wed, Feb 23, 2011 at 01:15:34AM -0700, Glenn Adams wrote:
> OK, understand on the junit headless issue. For checkstyle/findbugs it would
> be useful to fail the nightly build if they do not pass. I will investigate
> the necessary changes to enable this option, which I hope can be adopted.
I would not agree. Nightly builds are a courtesy to the user. It would
be good if we could guarantee that the builds pass the junit tests.
But it is not relevant to the user whether they pass checkstyle and
findbugs rules. These tests address the issue of code quality, not of
application quality.
Simon
> On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
>
> > On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
> > > I notice also that the nightly build target does not run all the junit
> > > tests. It would be better if it run all of them plus checkstyle and
> > > findbugs.
> >
> > Many junit tests require a display. Nightly builds are run in a
> > headless configuration, hence I had to disable many junit tests. At
> > nightly builds there is no one to check checkstyle and findbugs errors
> > and warnings; therefore there is no point in running them.
> >
> > Simon
> >
Re: junit tests in nightly builds [was: Re: Solving FindBugs issue]
Posted by Glenn Adams <gl...@skynav.com>.
OK, understand on the junit headless issue. For checkstyle/findbugs it would
be useful to fail the nightly build if they do not pass. I will investigate
the necessary changes to enable this option, which I hope can be adopted.
G.
On Wed, Feb 23, 2011 at 12:55 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
> On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
> > I notice also that the nightly build target does not run all the junit
> > tests. It would be better if it run all of them plus checkstyle and
> > findbugs.
>
> Many junit tests require a display. Nightly builds are run in a
> headless configuration, hence I had to disable many junit tests. At
> nightly builds there is no one to check checkstyle and findbugs errors
> and warnings; therefore there is no point in running them.
>
> Simon
>
junit tests in nightly builds [was: Re: Solving FindBugs issue]
Posted by Simon Pepping <sp...@leverkruid.eu>.
On Tue, Feb 22, 2011 at 11:25:20AM -0700, Glenn Adams wrote:
> I notice also that the nightly build target does not run all the junit
> tests. It would be better if it run all of them plus checkstyle and
> findbugs.
Many junit tests require a display. Nightly builds are run in a
headless configuration, hence I had to disable many junit tests. At
nightly builds there is no one to check checkstyle and findbugs errors
and warnings; therefore there is no point in running them.
Simon
Re: Solving FindBugs issue
Posted by Glenn Adams <gl...@skynav.com>.
Since the FOP project itself does not use any IDE for builds, then the ANT
build process should be considered the standard process for compilation,
junit tests, checkstyle tests, findbugs tests, etc.
I always run:
ant clean junit checkstyle findbugs
in order to verify no reported errors before I commit a change.
I notice also that the nightly build target does not run all the junit
tests. It would be better if it run all of them plus checkstyle and
findbugs.
G.
On Tue, Feb 22, 2011 at 12:34 AM, Simon Pepping <sp...@leverkruid.eu>wrote:
> When I build the project or part of it with Eclipse, and run findbugs
> afterwards (with ant), I get a number of errors. Now I always make a
> clean compile before running findbugs. I do not understand why Eclipse
> builds would create findbugs errors where a clean ant build does not.
> It makes findbugs seem fickle.
>
> I just tested it. With 'ant clean findbugs' I get 0 errors and
> warnings. With eclipse clean and build project, I get 23 low priority
> warnings.
>
> Simon
>
> On Mon, Feb 21, 2011 at 12:58:06PM -0700, Glenn Adams wrote:
> > The current trunk shows no warnings during ANT compile. Please make
> > reference to the current trunk/HEAD as 1.0 is published and history at
> this
> > time.
> >
> > It's a different matter with certain IDEs, e.g., Eclipse, which set their
> > warning levels to a more sensitive level than the ANT build.
> >
> > Although it would be nice to eliminate such additional warnings, it is
> not
> > as high priority IMO as ensuring that new compile, checkstyle, or
> findbugs
> > warnings/errors do not appear during ANT builds. At the same time,
> warnings
> > that do appear should not automatically be excluded without careful,
> manual
> > review.
> >
> > G.
> >
> > On Mon, Feb 21, 2011 at 12:49 PM, Eric Douglas <edouglas@blockhouse.com
> >wrote:
> >
> > > I haven't looked at the trunk lately but 1.0 has a ton of warnings, at
> > > least in my compile.
> > > I don't know how much warnings have changed over the versions.
> > > I think it was originally written to compile on Java 1.4 or maybe even
> > > 1.3.
> > > 1.5 shows thousands of warnings, 1.6 shows more.
> > > Some of the warnings are quirky (raw type list?), some are just
> wasteful
> > > (dead code? Local variable never referenced?).
> > > If there's code which is actually incorrect logic it needs to be fixed.
> > > If there's code which is just incomplete logic, maybe setting something
> > > up for a future improvement someone wanted to add, that should be
> > > removed and placed in a separate to do list document.
>
Re: Solving FindBugs issue
Posted by Simon Pepping <sp...@leverkruid.eu>.
When I build the project or part of it with Eclipse, and run findbugs
afterwards (with ant), I get a number of errors. Now I always make a
clean compile before running findbugs. I do not understand why Eclipse
builds would create findbugs errors where a clean ant build does not.
It makes findbugs seem fickle.
I just tested it. With 'ant clean findbugs' I get 0 errors and
warnings. With eclipse clean and build project, I get 23 low priority
warnings.
Simon
On Mon, Feb 21, 2011 at 12:58:06PM -0700, Glenn Adams wrote:
> The current trunk shows no warnings during ANT compile. Please make
> reference to the current trunk/HEAD as 1.0 is published and history at this
> time.
>
> It's a different matter with certain IDEs, e.g., Eclipse, which set their
> warning levels to a more sensitive level than the ANT build.
>
> Although it would be nice to eliminate such additional warnings, it is not
> as high priority IMO as ensuring that new compile, checkstyle, or findbugs
> warnings/errors do not appear during ANT builds. At the same time, warnings
> that do appear should not automatically be excluded without careful, manual
> review.
>
> G.
>
> On Mon, Feb 21, 2011 at 12:49 PM, Eric Douglas <ed...@blockhouse.com>wrote:
>
> > I haven't looked at the trunk lately but 1.0 has a ton of warnings, at
> > least in my compile.
> > I don't know how much warnings have changed over the versions.
> > I think it was originally written to compile on Java 1.4 or maybe even
> > 1.3.
> > 1.5 shows thousands of warnings, 1.6 shows more.
> > Some of the warnings are quirky (raw type list?), some are just wasteful
> > (dead code? Local variable never referenced?).
> > If there's code which is actually incorrect logic it needs to be fixed.
> > If there's code which is just incomplete logic, maybe setting something
> > up for a future improvement someone wanted to add, that should be
> > removed and placed in a separate to do list document.
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Andreas Delmelle <an...@telenet.be>.
On 21 Feb 2011, at 21:18, Eric Douglas wrote:
> I always had trouble with the ant build.
> Currently my ant build gets no errors and no warnings and creates fop.jar though it tells me junit and xmlunit are not found.
> Last time I put jars in my fop lib folder for junit and xmlunit it told me they were found and it wouldn't update the fop jar.
If there's already a set of precompiled classes, and none of the sources have changed, there is no need to update the jar. If you just run 'ant compile' or 'ant package', neither JUnit nor XMLUnit are required for a successful build. Compiler warnings are not errors, and as such do not prevent a correct build.
> How can I get hundreds or even thousands of warnings from trying to use the eclipse build project option with the default settings while the ant build has no errors?
The 'ant compile' task does show a hint of those warnings, IIC. Look for "Note: some classes use unchecked or unsafe operations."
If I were to guess, I'd say your IDE probably compiles with '-Xlint:unchecked', which displays all the gruesome details.
Regards,
Andreas
---
RE: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Eric Douglas <ed...@blockhouse.com>.
I always had trouble with the ant build.
Currently my ant build gets no errors and no warnings and creates
fop.jar though it tells me junit and xmlunit are not found.
Last time I put jars in my fop lib folder for junit and xmlunit it told
me they were found and it wouldn't update the fop jar.
How can I get hundreds or even thousands of warnings from trying to use
the eclipse build project option with the default settings while the ant
build has no errors? Is the ant build too permissive?
I don't know if all this junk (dead code?) is slowing it down or just
using up a little memory on loading the class but it should be cleaned
up though I could agree there are surely more important tasks.
________________________________
From: Glenn Adams [mailto:glenn@skynav.com]
Sent: Monday, February 21, 2011 2:58 PM
To: fop-dev@xmlgraphics.apache.org
Cc: Eric Douglas
Subject: Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
/xmlgraphics/fop/trunk: findbugs-exclude.xml
src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
The current trunk shows no warnings during ANT compile. Please make
reference to the current trunk/HEAD as 1.0 is published and history at
this time.
It's a different matter with certain IDEs, e.g., Eclipse, which set
their warning levels to a more sensitive level than the ANT build.
Although it would be nice to eliminate such additional warnings, it is
not as high priority IMO as ensuring that new compile, checkstyle, or
findbugs warnings/errors do not appear during ANT builds. At the same
time, warnings that do appear should not automatically be excluded
without careful, manual review.
G.
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
/xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Glenn Adams <gl...@skynav.com>.
The current trunk shows no warnings during ANT compile. Please make
reference to the current trunk/HEAD as 1.0 is published and history at this
time.
It's a different matter with certain IDEs, e.g., Eclipse, which set their
warning levels to a more sensitive level than the ANT build.
Although it would be nice to eliminate such additional warnings, it is not
as high priority IMO as ensuring that new compile, checkstyle, or findbugs
warnings/errors do not appear during ANT builds. At the same time, warnings
that do appear should not automatically be excluded without careful, manual
review.
G.
On Mon, Feb 21, 2011 at 12:49 PM, Eric Douglas <ed...@blockhouse.com>wrote:
> I haven't looked at the trunk lately but 1.0 has a ton of warnings, at
> least in my compile.
> I don't know how much warnings have changed over the versions.
> I think it was originally written to compile on Java 1.4 or maybe even
> 1.3.
> 1.5 shows thousands of warnings, 1.6 shows more.
> Some of the warnings are quirky (raw type list?), some are just wasteful
> (dead code? Local variable never referenced?).
> If there's code which is actually incorrect logic it needs to be fixed.
> If there's code which is just incomplete logic, maybe setting something
> up for a future improvement someone wanted to add, that should be
> removed and placed in a separate to do list document.
>
>
> -----Original Message-----
> From: Andreas Delmelle [mailto:andreas.delmelle@telenet.be]
> Sent: Monday, February 21, 2011 2:29 PM
> To: fop-dev@xmlgraphics.apache.org
> Subject: Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
> /xmlgraphics/fop/trunk: findbugs-exclude.xml
> src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
>
> On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
>
> > 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.
>
> I was about to convey a similar sentiment.
>
> If we are only going to ignore potential bugs, the point of the whole
> exercise seems totally lost.
>
> Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of
> those potentially introduced bugs (that we are now ignoring) is likely
> of the same nature as one that Chris fixed in the very same area a while
> back (potentially unclosed stream, leading to a descriptor leak in the
> AFP renderer)... Not very encouraging. :(
>
> Mea culpa:
> I saw one exclusion --unconfirmed cast-- that would seem to stem from my
> recent refactoring in the BlockStackingLMs. Not sure why an exclusion
> was chosen here, but adding an assert statement in the code avoids the
> warning as well *and* has the benefit of being visible exactly at the
> spot in the code where it applies. Seemed like the more proper way to
> handle this.
>
> Just my 2 cents...
>
>
> Regards,
>
> Andreas
> ---
>
>
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Andreas Delmelle <an...@telenet.be>.
On 21 Feb 2011, at 20:49, Eric Douglas wrote:
> I haven't looked at the trunk lately but 1.0 has a ton of warnings, at
> least in my compile.
> I don't know how much warnings have changed over the versions.
> I think it was originally written to compile on Java 1.4 or maybe even
> 1.3.
Even lower, IIRC. FOP ultimately dates back to last century.
> 1.5 shows thousands of warnings, 1.6 shows more.
> Some of the warnings are quirky (raw type list?),
Those are the 'unchecked assignment' type. On the one hand, not much to worry about, as it has posed little problems in the past. On the other, it did sometimes cause nasty ClassCastExceptions...
So we definitely try to eliminate those as we go. This number should reduce over time.
Regards,
Andreas
---
RE: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Eric Douglas <ed...@blockhouse.com>.
I haven't looked at the trunk lately but 1.0 has a ton of warnings, at
least in my compile.
I don't know how much warnings have changed over the versions.
I think it was originally written to compile on Java 1.4 or maybe even
1.3.
1.5 shows thousands of warnings, 1.6 shows more.
Some of the warnings are quirky (raw type list?), some are just wasteful
(dead code? Local variable never referenced?).
If there's code which is actually incorrect logic it needs to be fixed.
If there's code which is just incomplete logic, maybe setting something
up for a future improvement someone wanted to add, that should be
removed and placed in a separate to do list document.
-----Original Message-----
From: Andreas Delmelle [mailto:andreas.delmelle@telenet.be]
Sent: Monday, February 21, 2011 2:29 PM
To: fop-dev@xmlgraphics.apache.org
Subject: Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
/xmlgraphics/fop/trunk: findbugs-exclude.xml
src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
> 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.
I was about to convey a similar sentiment.
If we are only going to ignore potential bugs, the point of the whole
exercise seems totally lost.
Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of
those potentially introduced bugs (that we are now ignoring) is likely
of the same nature as one that Chris fixed in the very same area a while
back (potentially unclosed stream, leading to a descriptor leak in the
AFP renderer)... Not very encouraging. :(
Mea culpa:
I saw one exclusion --unconfirmed cast-- that would seem to stem from my
recent refactoring in the BlockStackingLMs. Not sure why an exclusion
was chosen here, but adding an assert statement in the code avoids the
warning as well *and* has the benefit of being visible exactly at the
spot in the code where it applies. Seemed like the more proper way to
handle this.
Just my 2 cents...
Regards,
Andreas
---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
/xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Glenn Adams <gl...@skynav.com>.
First, the point of the exclusion file is to "bless" warnings that, *by
design*, do not correspond with findbugs set of default warnings. The
presumption is that findbugs actually does identify real or potential bugs,
and there should be no argument about whether this is true or not.
What we are discussing here is the *automatic* addition of entries to the
exclusion file. In general, that is a *bad practice*, however, for
expediency purposes that has been done a few times already. Such practice
should not be continued.
Every exclusion added to the exclude file should be accompanied by a comment
in the exclude file, and perhaps a comment in the source file as well,
indicating why the exclusion occurred.
If folks are uncomfortable with marking an exclusion for an entire method,
then they can mark the specific line of source as well; however, that is
problematic when line numbers change. I feel it is better to mark it for the
method even if there is a risk it will unintentionally apply to another line
(in the same method) than was intended.
If folks make their methods shorter, then this reduces the likelihood of an
unintentional exclusion. My personal criteria is that, in the general case,
a method should occupy no more than one screen (on your favorite text
editor).
G.
On Mon, Feb 21, 2011 at 12:35 PM, Andreas Delmelle <
andreas.delmelle@telenet.be> wrote:
> On 21 Feb 2011, at 20:28, Andreas Delmelle wrote:
>
> > On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
> >
> >> 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.
> >
> > I was about to convey a similar sentiment.
> >
> > If we are only going to ignore potential bugs, the point of the whole
> exercise seems totally lost.
>
> Note also that, IIUC, if you define an exclusion for a given bug type in
> the scope of a given method, future devs are free to introduce many more
> instances of that very same bug type in that same method, without ever
> noticing that they are doing something wrong --even if they do their due
> diligence and run FB before each commit?
>
> That just doesn't seem right. :-/
>
>
> Regards,
>
> Andreas
> ---
>
>
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Andreas Delmelle <an...@telenet.be>.
On 21 Feb 2011, at 20:28, Andreas Delmelle wrote:
> On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
>
>> 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.
>
> I was about to convey a similar sentiment.
>
> If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost.
Note also that, IIUC, if you define an exclusion for a given bug type in the scope of a given method, future devs are free to introduce many more instances of that very same bug type in that same method, without ever noticing that they are doing something wrong --even if they do their due diligence and run FB before each commit?
That just doesn't seem right. :-/
Regards,
Andreas
---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Andreas Delmelle <an...@telenet.be>.
On 21 Feb 2011, at 22:18, Vincent Hennebert wrote:
> Did you remove the corresponding entry from the findbugs-exclude.xml
> file before running FindBugs again?
Locally, yes, and confirmed that the assert eliminated the warning. The exclusions file seems to be needing some more general cleanup, so I haven't committed that yet. 1012 exclusions is probably way too many already... :-/
Regards,
Andreas
---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk:
findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Vincent Hennebert <vh...@gmail.com>.
On 21/02/11 19:28, Andreas Delmelle wrote:
> On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
>
>> 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.
>
> I was about to convey a similar sentiment.
>
> If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost.
>
> Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :(
>
> Mea culpa:
> I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this.
Did you remove the corresponding entry from the findbugs-exclude.xml
file before running FindBugs again?
> Just my 2 cents...
>
>
> Regards,
>
> Andreas
Vincent
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Andreas Delmelle <an...@telenet.be>.
On 21 Feb 2011, at 19:15, Vincent Hennebert wrote:
> 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.
I was about to convey a similar sentiment.
If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost.
Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :(
Mea culpa:
I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this.
Just my 2 cents...
Regards,
Andreas
---
Re: Solving FindBugs issues
Posted by Vincent Hennebert <vh...@gmail.com>.
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, spepping@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
Re: Solving FindBugs issues
Posted by Simon Pepping <sp...@leverkruid.eu>.
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. It is much more encouraging to see that some of us
pick up the findbugs error report and use it to make code
improvements.
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, spepping@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
Re: Solving FindBugs issues
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 21:50, Andreas Delmelle wrote:
> ColorUtil seems to be somewhat of a can of worms...
<snip />
> If I judge correctly, for some (if not most) of the methods, there seems to be no need for a try-catch block (?)
Sorry, forgot RuntimeExceptions, of course, which are converted into PropertyExceptions by said catch-blocks...
Even then, explicitly catching RuntimeException as a last resort would seem preferable to plain Exception. That would have avoided my confusion here, at least. ;-)
Regards,
Andreas
---
Re: Solving FindBugs issues
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 20:39, Andreas Delmelle wrote:
> ...
> --and so, I caved in started already...
In PDFFactory, I have some conflicts to work out first, but the fix-up is rather simple there. In the respective methods, eliminate all unnecessary boxing and use explicit (double) casts where appropriate.
ColorUtil seems to be somewhat of a can of worms... It's not restricted to the method where the issue was raised, but likely the result of copy-pasting a pattern from a method that was already 'excused'.
At any rate, catching plain Exception should really be avoided, IMO, and traded for catching the specific type(s) of exception we know can be thrown in that block of code. In that case, it is completely unnecessary to resort to silly-looking constructs like catching PropertyException just to rethrow it (since the method allows it anyway).
If I judge correctly, for some (if not most) of the methods, there seems to be no need for a try-catch block (?)
3 remaining on my end ;-)
Regards,
Andreas
---
Re: Solving FindBugs issues
Posted by Andreas Delmelle <an...@telenet.be>.
On 22 Feb 2011, at 20:15, Vincent Hennebert wrote:
> <snip />
> Can we revert commit 1071912 and re-consider the issues one-by-one
> before ignoring them?
+1
As far as I can see, the raised warnings are really not so challenging that they cannot be addressed right away. Implementing an equals() method, removing unread fields and redundant checks...
I'm thinking, all a matter of minutes if you put your mind to it, no?
--and so, I caved in started already...
Regards,
Andreas
---
Re: Solving FindBugs issues
Posted by Vincent Hennebert <vh...@gmail.com>.
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?
> 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, spepping@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
Re: Solving FindBugs issues
Posted by Simon Pepping <sp...@leverkruid.eu>.
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.
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
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, spepping@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
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in
/xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Glenn Adams <gl...@skynav.com>.
Yes, there is are good reasons to continue using findbugs:
(1) it does find bugs
(2) they should not be added the exclude file without careful evaluation
(3) when evaluating, real bugs should be fixed; however, there are some
cases where findbugs reports a warning or error that is in fact not a bug,
but is intended to be that way by design; for example, returning a null vs
returning an empty array: findbugs will report the former as a warning, but
some designs may prefer that route;
Any exclusions added to the findbugs-exclude file should be done carefully
and after explicit evaluation.
I know that has not been the case recently, due to lack of time, and some
exclusions have been added without evaluation. This should be avoided, and
any exclusions added automatically should be marked as such for further
evaluation and repair.
G.
On Mon, Feb 21, 2011 at 11:15 AM, Vincent Hennebert <vh...@gmail.com>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, spepping@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
> >
> ==============================================================================
> > --- xmlgraphics/fop/trunk/findbugs-exclude.xml (original)
> > +++ xmlgraphics/fop/trunk/findbugs-exclude.xml Fri Feb 18 08:18:04 2011
> > @@ -5019,4 +5019,51 @@
> > <Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
> > </Match>
> > <!-- /Automatically generated list of exclusions -->
> > + <!-- Automatically generated list of exclusions on 18 February 2011
> -->
> > + <Match>
> > + <Class name="org.apache.fop.afp.goca.GraphicsSetProcessColor"/>
> > + <Method name="writeToStream"/>
> > + <Bug pattern="OS_OPEN_STREAM"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.pdf.PDFFactory"/>
> > + <Method name="makeSeparationColorSpace"/>
> > + <Bug pattern="DM_FP_NUMBER_CTOR"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.pdf.PDFFactory"/>
> > + <Method name="toColorVector"/>
> > + <Bug pattern="DM_FP_NUMBER_CTOR"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.fo.expr.PropertyTokenizer"/>
> > + <Field name="recognizeOperator"/>
> > + <Bug pattern="URF_UNREAD_FIELD"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.layoutmgr.BlockLayoutManager"/>
> > + <Method name="getNextChildElements"/>
> > + <Bug pattern="BC_UNCONFIRMED_CAST"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.util.ColorWithFallback"/>
> > + <!-- Listing the method 'equals' does not work -->
> > + <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.util.ColorWithFallback"/>
> > + <Method name="getAlternativeColors"/>
> > + <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.fo.expr.NamedColorFunction"/>
> > + <Method name="eval"/>
> > + <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
> > + </Match>
> > + <Match>
> > + <Class name="org.apache.fop.util.ColorUtil"/>
> > + <Method name="parseAsFopRgbNamedColor"/>
> > + <Bug pattern="REC_CATCH_EXCEPTION"/>
> > + </Match>
> > + <!-- /Automatically generated list of exclusions on 18 February 2011
> -->
> > </FindBugsFilter>
> > \ No newline at end of file
> >
> > Modified:
> xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
> > URL:
> http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java?rev=1071912&r1=1071911&r2=1071912&view=diff
> >
> ==============================================================================
> > ---
> xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
> (original)
> > +++
> xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
> Fri Feb 18 08:18:04 2011
> > @@ -60,7 +60,8 @@ public class PDFImageHandlerSVG implemen
> > private static Log log =
> LogFactory.getLog(PDFImageHandlerSVG.class);
> >
> > /** {@inheritDoc} */
> > - public void handleImage(RenderingContext context, Image image,
> Rectangle pos)
> > + public void handleImage(RenderingContext context, //
> CSOK: MethodLength
> > + Image image, Rectangle pos)
> > throws IOException {
> > PDFRenderingContext pdfContext = (PDFRenderingContext)context;
> > PDFContentGenerator generator = pdfContext.getGenerator();
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
> > For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
> >
>
Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk:
findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Posted by Vincent Hennebert <vh...@gmail.com>.
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, spepping@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
> ==============================================================================
> --- xmlgraphics/fop/trunk/findbugs-exclude.xml (original)
> +++ xmlgraphics/fop/trunk/findbugs-exclude.xml Fri Feb 18 08:18:04 2011
> @@ -5019,4 +5019,51 @@
> <Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
> </Match>
> <!-- /Automatically generated list of exclusions -->
> + <!-- Automatically generated list of exclusions on 18 February 2011 -->
> + <Match>
> + <Class name="org.apache.fop.afp.goca.GraphicsSetProcessColor"/>
> + <Method name="writeToStream"/>
> + <Bug pattern="OS_OPEN_STREAM"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.pdf.PDFFactory"/>
> + <Method name="makeSeparationColorSpace"/>
> + <Bug pattern="DM_FP_NUMBER_CTOR"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.pdf.PDFFactory"/>
> + <Method name="toColorVector"/>
> + <Bug pattern="DM_FP_NUMBER_CTOR"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.fo.expr.PropertyTokenizer"/>
> + <Field name="recognizeOperator"/>
> + <Bug pattern="URF_UNREAD_FIELD"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.layoutmgr.BlockLayoutManager"/>
> + <Method name="getNextChildElements"/>
> + <Bug pattern="BC_UNCONFIRMED_CAST"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.util.ColorWithFallback"/>
> + <!-- Listing the method 'equals' does not work -->
> + <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.util.ColorWithFallback"/>
> + <Method name="getAlternativeColors"/>
> + <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.fo.expr.NamedColorFunction"/>
> + <Method name="eval"/>
> + <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"/>
> + </Match>
> + <Match>
> + <Class name="org.apache.fop.util.ColorUtil"/>
> + <Method name="parseAsFopRgbNamedColor"/>
> + <Bug pattern="REC_CATCH_EXCEPTION"/>
> + </Match>
> + <!-- /Automatically generated list of exclusions on 18 February 2011 -->
> </FindBugsFilter>
> \ No newline at end of file
>
> Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java?rev=1071912&r1=1071911&r2=1071912&view=diff
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Fri Feb 18 08:18:04 2011
> @@ -60,7 +60,8 @@ public class PDFImageHandlerSVG implemen
> private static Log log = LogFactory.getLog(PDFImageHandlerSVG.class);
>
> /** {@inheritDoc} */
> - public void handleImage(RenderingContext context, Image image, Rectangle pos)
> + public void handleImage(RenderingContext context, // CSOK: MethodLength
> + Image image, Rectangle pos)
> throws IOException {
> PDFRenderingContext pdfContext = (PDFRenderingContext)context;
> PDFContentGenerator generator = pdfContext.getGenerator();
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
> For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org
>