commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Hendriks <D.Hendr...@tue.nl>
Subject Re: svn commit: r1139126 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/analysis/solvers/ site/xdoc/ test/java/org/apache/commons/math/analysis/ test/java/org/apache/commons/math/analysis/solvers/
Date Fri, 24 Jun 2011 09:33:58 GMT
 > I have 137 errors only adding the check for braces, and only a handful
 > without this check!

I used a trunk checkout and then:

mvn -Prc clean package
mvn checkstyle:checkstyle

and I got 22792 checkstyle errors... See attachments.

Dennis


Luc Maisonobe wrote:
> Le 24/06/2011 10:57, Dennis Hendriks a écrit :
>>  > No, we tend to fix really everything, at least at release time.
>>
>> I got this output this morning for trunk:
>>
>> [INFO] There are 22792 checkstyle errors.
> 
> I have 137 errors only adding the check for braces, and only a handful 
> without this check!
> 
> Do you use our checkstyle.xml file ?
> 
> Luc
> 
>> So there is a lot of work to do for the next release...
>>
>>  > If we
>>  > let warnings, it becomes difficult to sort real problems from false
>>  > positive. People have to take time to check every warning manually each
>>  > time, so allowing warnings really wastes more time in the long term that
>>  > it saves in the short term.
>>
>> I agree.
>>
>> Dennis
>>
>>
>> Luc Maisonobe wrote:
>>> Le 24/06/2011 09:09, Dennis Hendriks a écrit :
>>>> I agree with you that braces should always be used. Personally, I do
>>>> have one exception though, and that is if the statement following it is
>>>> on the same line. That way, it is one line, instead of 3 lines, which
>>>> makes it more readable. If the 'if' statement spans multiple lines, I
>>>> always include braces, even if there is only one single statement
>>>> involved. But that is just me...
>>> So we have at least two different opinions here, which is good. Let's
>>> see what other people think.
>>>
>>>> I think the rule is already active in Checkstyle. I think I just chose
>>>> to ignore it here.
>>> No, the rule is not active in our checkstyle configuration. I have
>>> tested it, there are about 130 violations of this rule in our code
>>> base, so it is really simple to fix all of them.
>>>
>>>> In general (not just this Checkstyle rule), is it allowed to ignore
>>>> checkstyle errors, if one believes that the code would be better if the
>>>> rule is ignored for a specific instance; that is, in case of false
>>>> positives?
>>> No, we tend to fix really everything, at least at release time. If we
>>> let warnings, it becomes difficult to sort real problems from false
>>> positive. People have to take time to check every warning manually
>>> each time, so allowing warnings really wastes more time in the long
>>> term that it saves in the short term.
>>>
>>> If a piece of code really needs to not comply to a rule, then we use
>>> the warning suppression mechanism around that piece of code. See at
>>> the end of our checkstyle configuration, there are already 5
>>> SuppressionCommentFilter set up.
>>>
>>> This also applies to findbugs, and we use findbugs-exclude-filter for
>>> false positives.
>>>
>>> Luc
>>>
>>>> Dennis
>>>>
>>>>
>>>> Luc Maisonobe wrote:
>>>>> Hi all,
>>>>>
>>>>> The provided patch for MATH-599 includes a number of no-brace if
>>>>> statements like the following ones:
>>>>>
>>>>>> + if (method == Method.ILLINOIS) f0 *= 0.5;
>>>>>> + if (method == Method.PEGASUS) f0 *= f1 / (f1 + fx);
>>>>> I'm not sure if we have an unwritten rule for this, but personally I
>>>>> dislike this style a lot. Checkstyle provides a NeedBraces rule to
>>>>> avoid this.
>>>>>
>>>>> What about activating this rule ?
>>>>>
>>>>> Luc
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


Mime
View raw message