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]
