"ant skip-code-checks" might be more discoverable (and less intimidating) than "ant unsafe". I'm a +100 on enabling more static analysis/linting/coverage reporting for full builds.
On Tue, Nov 8, 2022, 6:42 AM Josh McKenzie <jmcken...@apache.org> wrote: > We used to do linting years ago, and our approach was basically "the past > is past, but look for deltas of new bad things added and notify the > author". At least for me personally the signal / noise ratio was quite > valuable (and not just for my own code; i.e. this wasn't *just* a "Josh > Problem" ;) ) > > I was thinking about this w/the checkstyle additions; we need the ability > to bypass these easily for incremental change / test cycles while working > on something so devs don't have to pay the "analyze the entire code-base" > tax on each small local change. I'm a strong +1 on having these tools in > here and on by default for build or jar targets. > > Maybe it'd be more friendly to have something like "ant unsafe" (or "ant > jar-unsafe" so it's adjacent) that just does a quick jar w/out that stuff > so it's more discoverable than multiple -D params? > > On Tue, Nov 8, 2022, at 5:40 AM, Miklosovic, Stefan wrote: > > I confused Jacoco to be code analysis tool instead of code coverage one. > Early morning I guess :) I was thinking more about tooling like Sonar. What > do you think about that? Any pros / cons to Spotbugs? > > ________________________________________ > From: Miklosovic, Stefan <stefan.mikloso...@netapp.com> > Sent: Tuesday, November 8, 2022 11:36 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSSION] Add SpotBugs to the build > > Hi David, all > > what is the position of Jacoco in Cassandra as this point? I see it in > build.xml there are some targets and infrastructure around that. > > What is wrong with using Jacoco instead more heavily for static analysis? > Why do we need to introduce this? > > Regards > > ________________________________________ > From: David Capwell <dcapw...@apple.com> > Sent: Tuesday, November 8, 2022 0:10 > To: dev > Subject: [DISCUSSION] Add SpotBugs to the build > > NetApp Security WARNING: This is an external email. Do not click links or > open attachments unless you recognize the sender and know the content is > safe. > > > > > I was thinking that it would be good to add SpotBugs ( > https://spotbugs.github.io) into our build to help find bugs earlier in > the life cycle. SpotBugs is LGPL but as it is used only in the build and > not to within this project, then this should be fine with Apache. > > The motivation for adding this was from CASSANDRA-17178; the Simulator has > issues with Serializable classes missing serialVersionUID (as we deal with > ClassLoaders; this field is strongly recommend in general for all > Serializable classes), but this project can add more value as there are a > large collection of potential bugs to look out for; below are a few > examples found. > > * Number.valueOf vs Number.parse<size>. In many parts of the code we do > valueOf which returns a boxed value; we then unbox for the usage; this adds > more garbage that isn’t needed > * Using Number.compareTo rather than primitive compare functions (causing > boxing) > * Ignoring return value for functions that don’t have a side effect. This > happens in a few cases where we are building a StringBuilder where we call > .toString but ignore the string… then call it later on > * use of putIfAbsent without looking at the return. This was found in > CacheService where we add the SSTable reader to the cache and assume we win > the race and start using it… rather than using the object that won the race > > >
