hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Hsieh <...@cloudera.com>
Subject Re: [proposal] Findbugs to 0 policy in trunk once we get to findbugs 0. (HBASE-5589)
Date Tue, 27 Mar 2012 03:14:03 GMT
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 <jon@cloudera.com> wrote:

> Thanks for correcting this.  HBASE-5598 it is.
>
> Jon.
>
>
> On Sat, Mar 24, 2012 at 6:57 PM, lars hofhansl <lhofhansl@yahoo.com>wrote:
>
>> I think you mean HBASE-5598 ;)
>>
>>
>>
>> ________________________________
>>  From: Jonathan Hsieh <jon@cloudera.com>
>> To: dev@hbase.apache.org
>> 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
>> // jon@cloudera.com
>>
>
>
>
> --
> // Jonathan Hsieh (shay)
> // Software Engineer, Cloudera
> // jon@cloudera.com
>
>
>


-- 
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// jon@cloudera.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message