Re: Reducing the number of warnings in the codebase

2014-03-18 Thread Shawn Heisey
On 3/16/2014 2:48 PM, Uwe Schindler wrote: 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. Additional

RE: Reducing the number of warnings in the codebase

2014-03-18 Thread Uwe Schindler
On switch case fall-through: Some of them can be fixed with break statements, but a number of them are intentional. Is there a way to get the compiler to suppress the warning? I couldn't see one. Various IDEs have comment annotations that will cause the warnings to be ignored within the

Re: Reducing the number of warnings in the codebase

2014-03-17 Thread Furkan KAMACI
Hi; As being adviser of using Sonar I want to add some more points. First of all such kind of tools shows some metrics other than code warnings and we should show that metrics even we don't use them. In example *sometimes *code complexity is a good measure to review your code. I should mention

Reducing the number of warnings in the codebase

2014-03-16 Thread Shawn Heisey
With the default settings in Eclipse, the Lucene/Solr codebase shows over 6000 warnings. This is the case for both branch_4x and trunk. I'm no expert, but this does seem a little excessive. If I were to take on the task of reducing this number, what advice can the group give me? Is there

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Furkan KAMACI
Hi; I've run FindBugs for Lucene/Solr project. If you use Intellij IDEA you can group the warnings according to their importance. I've opened issues and attached patches for top level warnings/errors (and some others) that FindBugs found. On the other hand I have another suggestion for

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Ahmet Arslan
Hi Shawn,  +1 for the idea, we should take full advantage of Eclipse, IntelliJ etc. Here are some relevant tickets created by Furkan. https://issues.apache.org/jira/browse/LUCENE-5506 https://issues.apache.org/jira/browse/SOLR-5838 https://issues.apache.org/jira/browse/SOLR-5839 I believe 

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Michael McCandless
On Sun, Mar 16, 2014 at 6:09 AM, Furkan KAMACI furkankam...@gmail.com wrote: I've run FindBugs for Lucene/Solr project. If you use Intellij IDEA you can group the warnings according to their importance. I've opened issues and attached patches for top level warnings/errors (and some others)

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Furkan KAMACI
hİ; Thanks Michael. I will open a Jira issue for it. Thanks; Furkan KAMACI 2014-03-16 13:35 GMT+02:00 Michael McCandless luc...@mikemccandless.com: On Sun, Mar 16, 2014 at 6:09 AM, Furkan KAMACI furkankam...@gmail.com wrote: I've run FindBugs for Lucene/Solr project. If you use Intellij

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Furkan KAMACI
Hi; Here is Apache projects that is analyzed via Sonar: https://analysis.apache.org/all_projects?qualifier=TRK Thanks; Furkan KAMACI 2014-03-16 15:37 GMT+02:00 Furkan KAMACI furkankam...@gmail.com: hİ; Thanks Michael. I will open a Jira issue for it. Thanks; Furkan KAMACI 2014-03-16

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Shawn Heisey
On 3/16/2014 5:35 AM, Michael McCandless wrote: On Sun, Mar 16, 2014 at 6:09 AM, Furkan KAMACI furkankam...@gmail.com wrote: I've run FindBugs for Lucene/Solr project. If you use Intellij IDEA you can group the warnings according to their importance. I've opened issues and attached patches

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Shawn Heisey
On 3/16/2014 12:26 PM, Shawn Heisey wrote: Would it be too much administrative @#!* to create an umbrella issue? I'd suggest LUCENE-5130 for this purpose, except that I'm not 100% positive that failing the build is the right answer. I fully understand the motivation ... it would certainly

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Benson Margulies
Just because some tool expresses distaste, doesn't imply that everyone here agrees that it's a problem we should fix. 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

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Ahmet Arslan
Hi, Here are some rules : Following String methods where left hand side is empty.   String.replace()   String.toUpperCase()   String.toLowerCase()   String.replaceFirst()   String.trim() In test cases (subblasses of SolrTestCaseJ4) methods without assertU(). see : SOLR-5685    adoc()  

RE: Reducing the number of warnings in the codebase

2014-03-16 Thread Uwe Schindler
String.toUpperCase() and String.toLowerCase() without Locale. see : SOLR- 2281 and LUCENE-2466 Those are already detected by forbidden-apis. Can ant precommit/forbidden-apis be used to detect above? Ahmet On Sunday, March 16, 2014 9:53 PM, Benson Margulies bimargul...@gmail.com

RE: Reducing the number of warnings in the codebase

2014-03-16 Thread Uwe Schindler
Hi, 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

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Ahmet Arslan
Hi Uwe, I looked for definitions under lucene/tools/forbiddenApis/*.txt files but I couldn't find. Where are those rule are defined? I am wondering about the syntax, can you point? Thanks, Ahmet On Sunday, March 16, 2014 10:40 PM, Uwe Schindler u...@thetaphi.de wrote: String.toUpperCase()

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Shawn Heisey
On 3/16/2014 1:52 PM, Benson Margulies wrote: Just because some tool expresses distaste, doesn't imply that everyone here agrees that it's a problem we should fix. I had most of a reply written before I saw Uwe's response. He brings up some good points that made me re-think important parts of

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Uwe Schindler
That are the default rules in the jar file. All so called unsafe methods and classes. The list is immense. http://code.google.com/p/forbidden-apis/source/browse/trunk#trunk%2Fsrc%2Fmain%2Fresources%2Fde%2Fthetaphi%2Fforbiddenapis%2Fsignatures See also homepage of tool and its jar file. Uwe

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Shawn Heisey
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:

Re: Reducing the number of warnings in the codebase

2014-03-16 Thread Benson Margulies
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

RE: Reducing the number of warnings in the codebase

2014-03-16 Thread Robert Muir
I agree with uwe. Start with javac, use -Werror flag and fix those first. Stupid warnings like serialization are already disabled for javac in the build. On Mar 16, 2014 4:48 PM, Uwe Schindler u...@thetaphi.de wrote: Hi, Just because some tool expresses distaste, doesn't imply that everyone