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
