HBase, mail # dev - Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

[proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Jonathan Hsieh 2012-03-25, 01:38

Hey all,

I brought this up in jira (HBASE-5589) and stack suggested posting to dev@,
so here's a proposal

Currently we are somewhere around the 770 warnings/errors mark on trunk,
and our hadoopqa bot will soon start complaining again as code gets checked
in. Since many patches in flight and since it may be a bit onerous for
committers to enforce a no new findbugs policy right away, what do you all
think about committers enforcing a no-new-findbugs errors policy on
reviews once
this we finally get the findbugs warnings to 0?

To knock down the findbugs violation number, we should probably chop this
into subtasks to break down the workload. Ideally we'd first fix
warnings/errors, and then for remaining spurious warnings (like System.exit
in some tools), we use an excludes file as opposed to marking up code with
findbugs-specific annotations since this may require the inclusion of a GPL
licensed jar. (On a previous project, the findbugs annotation jar was GPL'd
which causes license problems, maybe different now).

Sound good?

Jon

--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Mubarak Seyed 2012-03-26, 18:35

Please feel free to give me a subset of the work as well. Thanks.

--Mubarak

On Mar 25, 2012, at 10:06 PM, Laxman wrote:

> I agree with you Jon. Feel free to assign me some subtasks.
>
> --
> Regards,
> Laxman
>> -----Original Message-----
>> From: Gregory Chanan [mailto:[EMAIL PROTECTED]]
>> Sent: Monday, March 26, 2012 6:29 AM
>> To: [EMAIL PROTECTED]
>> Subject: Re: [proposal] Findbugs to 0 policy in trunk once we get to
>> findbugs 0. (HBASE-5589)
>>
>> Sounds good, feel free to give me a subset of the work as well.
>>
>> Greg
>>
>> On Sat, Mar 24, 2012 at 9:52 PM, Stack <[EMAIL PROTECTED]> wrote:
>>
>>> On Sat, Mar 24, 2012 at 6:38 PM, Jonathan Hsieh <[EMAIL PROTECTED]>
>> wrote:
>>>> Sound good?
>>>>
>>>
>>> Yes. You can give me a subset to work through.
>>>
>>> Ditto for javadoc warnings.
>>>
>>> Good stuff Jon,
>>>
>>> St.Ack
>>>
>

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Dave Wang 2012-03-26, 18:43

What the heck, I'll throw my name into the ring as well. :D

- Dave

On Mon, Mar 26, 2012 at 11:35 AM, Mubarak Seyed <[EMAIL PROTECTED]> wrote:

> Please feel free to give me a subset of the work as well. Thanks.
>
> --Mubarak
>
> On Mar 25, 2012, at 10:06 PM, Laxman wrote:
>
> > I agree with you Jon. Feel free to assign me some subtasks.
> >
> > --
> > Regards,
> > Laxman
> >> -----Original Message-----
> >> From: Gregory Chanan [mailto:[EMAIL PROTECTED]]
> >> Sent: Monday, March 26, 2012 6:29 AM
> >> To: [EMAIL PROTECTED]
> >> Subject: Re: [proposal] Findbugs to 0 policy in trunk once we get to
> >> findbugs 0. (HBASE-5589)
> >>
> >> Sounds good, feel free to give me a subset of the work as well.
> >>
> >> Greg
> >>
> >> On Sat, Mar 24, 2012 at 9:52 PM, Stack <[EMAIL PROTECTED]> wrote:
> >>
> >>> On Sat, Mar 24, 2012 at 6:38 PM, Jonathan Hsieh <[EMAIL PROTECTED]>
> >> wrote:
> >>>> Sound good?
> >>>>
> >>>
> >>> Yes. You can give me a subset to work through.
> >>>
> >>> Ditto for javadoc warnings.
> >>>
> >>> Good stuff Jon,
> >>>
> >>> St.Ack
> >>>
> >
>
>

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

lars hofhansl 2012-03-25, 01:57

I think you mean HBASE-5598 ;)

________________________________
From: Jonathan Hsieh <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED]
Sent: Saturday, March 24, 2012 6:38 PM
Subject: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Hey all,

I brought this up in jira (HBASE-5589) and stack suggested posting to dev@,
so here's a proposal

Currently we are somewhere around the 770 warnings/errors mark on trunk,
and our hadoopqa bot will soon start complaining again as code gets checked
in. Since many patches in flight and since it may be a bit onerous for
committers to enforce a no new findbugs policy right away, what do you all
think about committers enforcing a no-new-findbugs errors policy on
reviews once
this we finally get the findbugs warnings to 0?

To knock down the findbugs violation number, we should probably chop this
into subtasks to break down the workload. Ideally we'd first fix
warnings/errors, and then for remaining spurious warnings (like System.exit
in some tools), we use an excludes file as opposed to marking up code with
findbugs-specific annotations since this may require the inclusion of a GPL
licensed jar. (On a previous project, the findbugs annotation jar was GPL'd
which causes license problems, maybe different now).

Sound good?

Jon

--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Jonathan Hsieh 2012-03-25, 02:21

Thanks for correcting this. HBASE-5598 it is.

Jon.

On Sat, Mar 24, 2012 at 6:57 PM, lars hofhansl <[EMAIL PROTECTED]> wrote:

> I think you mean HBASE-5598 ;)
>
>
>
> ________________________________
> From: Jonathan Hsieh <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Sent: Saturday, March 24, 2012 6:38 PM
> Subject: [proposal] Findbugs to 0 policy in trunk once we get to findbugs
> 0. (HBASE-5589)
>
> Hey all,
>
> I brought this up in jira (HBASE-5589) and stack suggested posting to dev@
> ,
> so here's a proposal
>
> Currently we are somewhere around the 770 warnings/errors mark on trunk,
> and our hadoopqa bot will soon start complaining again as code gets checked
> in. Since many patches in flight and since it may be a bit onerous for
> committers to enforce a no new findbugs policy right away, what do you all
> think about committers enforcing a no-new-findbugs errors policy on
> reviews once
> this we finally get the findbugs warnings to 0?
>
> To knock down the findbugs violation number, we should probably chop this
> into subtasks to break down the workload. Ideally we'd first fix
> warnings/errors, and then for remaining spurious warnings (like System.exit
> in some tools), we use an excludes file as opposed to marking up code with
> findbugs-specific annotations since this may require the inclusion of a GPL
> licensed jar. (On a previous project, the findbugs annotation jar was GPL'd
> which causes license problems, maybe different now).
>
> Sound good?
>
> Jon
>
> --
> // Jonathan Hsieh (shay)
> // Software Engineer, Cloudera
> // [EMAIL PROTECTED]
>

--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Jonathan Hsieh 2012-03-27, 03:14

Hey all,

Right now findbugs is only run on the precommit builds -- I'm basing this
off of a recent one:

https://builds.apache.org/job/PreCommit-HBASE-Build/1297//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html

The results are slightly different if you run 'mvn findbugs:findbugs' a la
http://mojo.codehaus.org/findbugs-maven-plugin/plugin-info.html .

I chopped it up into about 15 groups -- so between the 5-6 interested you
can just pick 3 or so. I think it probably makes sense to knock out a
category at a time to make reviewing easier. Ideally each of these would
edit the number of allowed findbugs errors in
dev-support/test-patch.properties.

I'll file these as sub-tasks of HBASE-5598 shortly. Most look mechanical,
but a few likely need some thought. I'll start off by taking the thrift
exclude.

Bad Practice:
* [findbugs] Exclude thrift warnings [CN] (Jon)
* [findbugs] Fix compareTo/equals/hashcode warnings [Eq,ES, HE]
* [findbugs] Fix Null pointers [NP]
* [findbugs] Exclude (?) protobuf warnings [CN, Se]
* [findbugs] Fix the rest of Bad practice.

Correctness:
* [findbugs] Fix correctness warnings

Experimental: (Maybe break down by packages).
* [findbugs] Investigate experimental warnings.

Malicious code:
* [findbugs] Fix "exposed internal representation" (many are likely exclude
for perf reasons) [EI,EI2]
* [findbugs] Fix finals/protected/constants [MS]

Security
* [findbugs] Fix security warning

Multithreaded:
* [findbugs] Fix extra synchronization on CSLM's, Atomic* [JLM]
* [findbugs] Fix wait/notify synchronization/inconsistency sync [IS,LI,
MWM, NN, SWL, UG,UW]
* [findbugs] Fix lock release on all paths [UL]

Performance:
* [findbugs] Fix perf warnings

Dodgy: (maybe break down by packages)
* [findbugs] Fix Dodgy bugs

Feels like its time to eat yer vegetables.

Jon.
On Sat, Mar 24, 2012 at 7:21 PM, Jonathan Hsieh <[EMAIL PROTECTED]> wrote:

> Thanks for correcting this. HBASE-5598 it is.
>
> Jon.
>
>
> On Sat, Mar 24, 2012 at 6:57 PM, lars hofhansl <[EMAIL PROTECTED]>wrote:
>
>> I think you mean HBASE-5598 ;)
>>
>>
>>
>> ________________________________
>> From: Jonathan Hsieh <[EMAIL PROTECTED]>
>> To: [EMAIL PROTECTED]
>> Sent: Saturday, March 24, 2012 6:38 PM
>> Subject: [proposal] Findbugs to 0 policy in trunk once we get to findbugs
>> 0. (HBASE-5589)
>>
>> Hey all,
>>
>> I brought this up in jira (HBASE-5589) and stack suggested posting to dev@
>> ,
>> so here's a proposal
>>
>> Currently we are somewhere around the 770 warnings/errors mark on trunk,
>> and our hadoopqa bot will soon start complaining again as code gets
>> checked
>> in. Since many patches in flight and since it may be a bit onerous for
>> committers to enforce a no new findbugs policy right away, what do you all
>> think about committers enforcing a no-new-findbugs errors policy on
>> reviews once
>> this we finally get the findbugs warnings to 0?
>>
>> To knock down the findbugs violation number, we should probably chop this
>> into subtasks to break down the workload. Ideally we'd first fix
>> warnings/errors, and then for remaining spurious warnings (like
>> System.exit
>> in some tools), we use an excludes file as opposed to marking up code with
>> findbugs-specific annotations since this may require the inclusion of a
>> GPL
>> licensed jar. (On a previous project, the findbugs annotation jar was
>> GPL'd
>> which causes license problems, maybe different now).
>>
>> Sound good?
>>
>> Jon
>>
>> --
>> // Jonathan Hsieh (shay)
>> // Software Engineer, Cloudera
>> // [EMAIL PROTECTED]
>>
>
>
>
> --
> // Jonathan Hsieh (shay)
> // Software Engineer, Cloudera
> // [EMAIL PROTECTED]
>
>
>
--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]

Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)

Jonathan Hsieh 2012-03-28, 16:20

I've split tasks now -- you can add yourselves here now:

https://issues.apache.org/jira/browse/HBASE-5598

jon.
On Mon, Mar 26, 2012 at 8:14 PM, Jonathan Hsieh <[EMAIL PROTECTED]> wrote:

> Hey all,
>
> Right now findbugs is only run on the precommit builds -- I'm basing this
> off of a recent one:
>
>
> https://builds.apache.org/job/PreCommit-HBASE-Build/1297//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
>
> The results are slightly different if you run 'mvn findbugs:findbugs' a
> la http://mojo.codehaus.org/findbugs-maven-plugin/plugin-info.html .
>
> I chopped it up into about 15 groups -- so between the 5-6 interested you
> can just pick 3 or so. I think it probably makes sense to knock out a
> category at a time to make reviewing easier. Ideally each of these would
> edit the number of allowed findbugs errors in
> dev-support/test-patch.properties.
>
> I'll file these as sub-tasks of HBASE-5598 shortly. Most look mechanical,
> but a few likely need some thought. I'll start off by taking the thrift
> exclude.
>
> Bad Practice:
> * [findbugs] Exclude thrift warnings [CN] (Jon)
> * [findbugs] Fix compareTo/equals/hashcode warnings [Eq,ES, HE]
> * [findbugs] Fix Null pointers [NP]
> * [findbugs] Exclude (?) protobuf warnings [CN, Se]
> * [findbugs] Fix the rest of Bad practice.
>
> Correctness:
> * [findbugs] Fix correctness warnings
>
> Experimental: (Maybe break down by packages).
> * [findbugs] Investigate experimental warnings.
>
> Malicious code:
> * [findbugs] Fix "exposed internal representation" (many are likely
> exclude for perf reasons) [EI,EI2]
> * [findbugs] Fix finals/protected/constants [MS]
>
> Security
> * [findbugs] Fix security warning
>
> Multithreaded:
> * [findbugs] Fix extra synchronization on CSLM's, Atomic* [JLM]
> * [findbugs] Fix wait/notify synchronization/inconsistency sync [IS,LI,
> MWM, NN, SWL, UG,UW]
> * [findbugs] Fix lock release on all paths [UL]
>
> Performance:
> * [findbugs] Fix perf warnings
>
> Dodgy: (maybe break down by packages)
> * [findbugs] Fix Dodgy bugs
>
> Feels like its time to eat yer vegetables.
>
> Jon.
>
>
> On Sat, Mar 24, 2012 at 7:21 PM, Jonathan Hsieh <[EMAIL PROTECTED]> wrote:
>
>> Thanks for correcting this. HBASE-5598 it is.
>>
>> Jon.
>>
>>
>> On Sat, Mar 24, 2012 at 6:57 PM, lars hofhansl <[EMAIL PROTECTED]>wrote:
>>
>>> I think you mean HBASE-5598 ;)
>>>
>>>
>>>
>>> ________________________________
>>> From: Jonathan Hsieh <[EMAIL PROTECTED]>
>>> To: [EMAIL PROTECTED]
>>> Sent: Saturday, March 24, 2012 6:38 PM
>>> Subject: [proposal] Findbugs to 0 policy in trunk once we get to
>>> findbugs 0. (HBASE-5589)
>>>
>>> Hey all,
>>>
>>> I brought this up in jira (HBASE-5589) and stack suggested posting to
>>> dev@,
>>> so here's a proposal
>>>
>>> Currently we are somewhere around the 770 warnings/errors mark on trunk,
>>> and our hadoopqa bot will soon start complaining again as code gets
>>> checked
>>> in. Since many patches in flight and since it may be a bit onerous for
>>> committers to enforce a no new findbugs policy right away, what do you
>>> all
>>> think about committers enforcing a no-new-findbugs errors policy on
>>> reviews once
>>> this we finally get the findbugs warnings to 0?
>>>
>>> To knock down the findbugs violation number, we should probably chop this
>>> into subtasks to break down the workload. Ideally we'd first fix
>>> warnings/errors, and then for remaining spurious warnings (like
>>> System.exit
>>> in some tools), we use an excludes file as opposed to marking up code
>>> with
>>> findbugs-specific annotations since this may require the inclusion of a
>>> GPL
>>> licensed jar. (On a previous project, the findbugs annotation jar was
>>> GPL'd
>>> which causes license problems, maybe different now).
>>>
>>> Sound good?
>>>
>>> Jon
>>>
>>> --
>>> // Jonathan Hsieh (shay)
>>> // Software Engineer, Cloudera
>>> // [EMAIL PROTECTED]
>>>
>>
>>
>>
>> --
>> // Jonathan Hsieh (shay)
>> // Software Engineer, Cloudera
>> // [EMAIL PROTECTED]
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]