> 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.
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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
