I think we avoid bikeshed by making incremental changes. If you offer a commit to turn off serial version UID whining, I'll +1 it. And then we iterate, in small doses, agreeing to either spike the warning or change the code.
In passing, I will warn you that the IDEs can be very stubborn; in some cases, there is no way to avoid some amount of whining. Eclipse used to insist on warning on every @SuppressWarnings that it didn't understand. It might still. On Sun, Mar 16, 2014 at 5:29 PM, Shawn Heisey <s...@elyograg.org> wrote: > A starting comment: We could bikeshed for *years*. > > General thought: The more I think about it, the more I like the notion > of confining most of the cleanup to trunk. Actual bug fixes and changes > that are relatively non-invasive should be backported. > > On 3/16/2014 2:48 PM, Uwe Schindler wrote: >>> Just because some tool expresses distaste, doesn't imply that everyone here >>> agrees that it's a problem we should fix. >> >> Yes that is my biggest problem. Lots of warnings by Eclipse are just >> bullshit because of the code style in Lucene and for example the way we do >> things - e.g., it complains about missing close() all the time, just because >> we use IOUtils.closeWhileHandlingExceptions() for that. > > My original thought on this was that we should use a combination of > SuppressWarnings and actual code changes to eliminate most of the > warnings that show up in the well-supported IDEs when they are > configured with *default* settings. > > Uwe brings up a really good point that there are a number of completely > useless warnings, but I think there's still value in looking through > EVERY default IDE warning and evaluating each one on a case-by-case > basis to decide whether that specific warning should be fixed or > ignored. It could be a sort of background task with an open Jira for > tracking commits. It could also be something that we decide isn't worth > the effort. > >>> In my experience, the default Sonar rulesets contain many things that people >>> here are prone to disagree with. Start with serialVersionUID: >>> do we care? Why would we care? In what cases to we really believe that a >>> sane person would be using Java serialization with a Lucene/Solr class? >> >> We officially don't support serialization, so all warnings are useless. It's >> just Eclipse that complains for no reason. > > Project-specific IDE settings for errors/warnings (set by the ant build > target) will go a long way towards making the whole situation better. > For the current stable branch, we should include settings for anything > that we want to ignore on trunk, but only a subset of those problems > that get elevated to error status. > >>> Sonar can also be a bit cranky; it arranges for various tools to run via >>> mechanisms that sometimes conflict with the ways you might run them >>> yourself. >>> >>> So I'd suggest a process like: >>> >>> 1. Someone proposes a set of (e.g.) checkstyle rules to live by. >>> 2. That ruleset is refined by experiment. >>> 3. We make violations fail the build. >>> >>> Then lather, rinse, repeat for other tools. >> >> Yes I agree. I am strongly against PMD or CheckStyle without our own rules. >> Forbiddeen-apis was invented because of the brokenness of PMD and CheckStyle >> to detect default Locale/Charset/Timezone violations (and also because those >> tools are slow). >> We should better fix out Eclipse Project generate to hide the warnings that >> are just wrong. >> >> I would prefer: Before we fix warnings by 3rd party tools like Eclipse, we >> should first fix only the warnings emitted by Javac. The others are just >> unimportant to me and I don't want to fix those which are just wrong for our >> code style. >> >> We already have ECJ in our build (to lint javadocs), we can make some >> Eclipse warnings fatal through the ecj config file in our SVN, to fail build >> on some warnings. I disagree with using PMD or Checkstyle, those tools are >> uncomplete and broken, sorry. > > +1 all around. I want to eliminate all the noise. If we had IDE > warnings measured in dozens instead of thousands, it would be a useful > data point that wouldn't get ignored. > > Thanks, > Shawn > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
