You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Luc Maisonobe <Lu...@free.fr> on 2011/06/24 09:02:22 UTC
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/
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
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/
Posted by Sébastien Brisard <se...@m4x.org>.
+1
2011/6/24 Luc Maisonobe <Lu...@free.fr>:
> 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
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/
Posted by James Carman <ja...@carmanconsulting.com>.
Unless we've specifically set up our pom.xml file to handle it, the
release plugin doesn't work too well with Maven3. Do you have a copy
of Maven2 lying around?
On Mon, Jun 27, 2011 at 4:13 AM, Dennis Hendriks <D....@tue.nl> wrote:
> $ mvn -v
> Apache Maven 3.0.3 (r1075438; 2011-02-28 18:31:09+0100)
> Maven home: /mnt/run/dhendriks/apps/apache-maven
> Java version: 1.6.0_25, vendor: Sun Microsystems Inc.
> Java home: /usr/java/jdk1.6.0_25/jre
> Default locale: en_US, platform encoding: UTF-8
> OS name: "linux", version: "2.6.18-128.1.6.el5pae", arch: "i386", family:
> "unix"
>
> Dennis
>
> luc.maisonobe@free.fr wrote:
>>
>> ----- "Dennis Hendriks" <D....@tue.nl> a écrit :
>>
>>> I tried 'mvn clean site', and got this:
>>>
>>> (...)
>>> main:
>>> [INFO] Executed tasks
>>> [INFO]
>>> [INFO] --- maven-remote-resources-plugin:1.0:process (default) @
>>> commons-math ---
>>> [INFO]
>>> [INFO] --- maven-resources-plugin:2.4.1:resources (default-resources)
>>> @ commons-math ---
>>> [INFO] Using 'UTF-8' encoding to copy filtered resources.
>>> [INFO] Copying 2 resources to META-INF
>>> [INFO] Copying 1 resource to META-INF/localization
>>> [INFO]
>>> [INFO] --- maven-compiler-plugin:2.1:compile (default-compile) @
>>> commons-math ---
>>> [INFO] Nothing to compile - all classes are up to date
>>> [INFO]
>>> [INFO] <<< jdepend-maven-plugin:2.0-beta-2:generate (report:generate)
>>> @ commons-math <<<
>>> [INFO] configuring report plugin
>>> org.codehaus.mojo:rat-maven-plugin:1.0-alpha-3
>>> [INFO] configuring report plugin
>>> org.codehaus.mojo:findbugs-maven-plugin:2.1
>>> [INFO]
>>> ------------------------------------------------------------------------
>>> [INFO] BUILD FAILURE
>>> [INFO]
>>> ------------------------------------------------------------------------
>>> [INFO] Total time: 1:05.413s
>>> [INFO] Finished at: Mon Jun 27 08:54:24 CEST 2011
>>> [INFO] Final Memory: 32M/280M
>>> [INFO]
>>> ------------------------------------------------------------------------
>>> [ERROR] Failed to execute goal
>>> org.apache.maven.plugins:maven-site-plugin:3.0-beta-3:site
>>> (default-site) on project commons-math: failed to get Reports: Unable to
>>> parse configuration of mojo
>>> org.codehaus.mojo:findbugs-maven-plugin:2.1:findbugs for parameter
>>> localRepository: Abstract class or interface
>>> 'org.apache.maven.artifact.repository.DefaultArtifactRepository'
>>> cannot be instantiated -> [Help 1]
>>> [ERROR]
>>> [ERROR] To see the full stack trace of the errors, re-run Maven with
>>> the -e switch.
>>> [ERROR] Re-run Maven using the -X switch to enable full debug
>>> logging.
>>> [ERROR]
>>> [ERROR] For more information about the errors and possible solutions,
>>>
>>> please read the following articles:
>>> [ERROR] [Help 1]
>>> http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
>>>
>>> any ideas?
>>
>> No ideas at all :-(
>> Which version of maven do you use ?
>>
>> Luc
>>
>>> Dennis
>>>
>>>
>>> Luc Maisonobe wrote:
>>>>
>>>> Le 24/06/2011 11:33, Dennis Hendriks a écrit :
>>>>>
>>>>> > 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.
>>>>
>>>> The checkstyle file used is not the one from Apache Commons Math.
>>>
>>> I'm
>>>>
>>>> not sure using the "rc" profile in the -Prc flag really cleaned
>>>> everything on the first command, so the second command "mvn
>>>> checkstyle:checkstyle" did use a cached value.
>>>>
>>>> Try something along these lines, without any -P flag:
>>>>
>>>> mvn clean site
>>>>
>>>> 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
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/
Posted by Dennis Hendriks <D....@tue.nl>.
$ mvn -v
Apache Maven 3.0.3 (r1075438; 2011-02-28 18:31:09+0100)
Maven home: /mnt/run/dhendriks/apps/apache-maven
Java version: 1.6.0_25, vendor: Sun Microsystems Inc.
Java home: /usr/java/jdk1.6.0_25/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "2.6.18-128.1.6.el5pae", arch: "i386", family:
"unix"
Dennis
luc.maisonobe@free.fr wrote:
> ----- "Dennis Hendriks" <D....@tue.nl> a écrit :
>
>> I tried 'mvn clean site', and got this:
>>
>> (...)
>> main:
>> [INFO] Executed tasks
>> [INFO]
>> [INFO] --- maven-remote-resources-plugin:1.0:process (default) @
>> commons-math ---
>> [INFO]
>> [INFO] --- maven-resources-plugin:2.4.1:resources (default-resources)
>> @
>> commons-math ---
>> [INFO] Using 'UTF-8' encoding to copy filtered resources.
>> [INFO] Copying 2 resources to META-INF
>> [INFO] Copying 1 resource to META-INF/localization
>> [INFO]
>> [INFO] --- maven-compiler-plugin:2.1:compile (default-compile) @
>> commons-math ---
>> [INFO] Nothing to compile - all classes are up to date
>> [INFO]
>> [INFO] <<< jdepend-maven-plugin:2.0-beta-2:generate (report:generate)
>> @
>> commons-math <<<
>> [INFO] configuring report plugin
>> org.codehaus.mojo:rat-maven-plugin:1.0-alpha-3
>> [INFO] configuring report plugin
>> org.codehaus.mojo:findbugs-maven-plugin:2.1
>> [INFO]
>> ------------------------------------------------------------------------
>> [INFO] BUILD FAILURE
>> [INFO]
>> ------------------------------------------------------------------------
>> [INFO] Total time: 1:05.413s
>> [INFO] Finished at: Mon Jun 27 08:54:24 CEST 2011
>> [INFO] Final Memory: 32M/280M
>> [INFO]
>> ------------------------------------------------------------------------
>> [ERROR] Failed to execute goal
>> org.apache.maven.plugins:maven-site-plugin:3.0-beta-3:site
>> (default-site)
>> on project commons-math: failed to get Reports: Unable to parse
>> configuration of mojo
>> org.codehaus.mojo:findbugs-maven-plugin:2.1:findbugs
>> for parameter localRepository: Abstract class or interface
>> 'org.apache.maven.artifact.repository.DefaultArtifactRepository'
>> cannot be
>> instantiated -> [Help 1]
>> [ERROR]
>> [ERROR] To see the full stack trace of the errors, re-run Maven with
>> the -e
>> switch.
>> [ERROR] Re-run Maven using the -X switch to enable full debug
>> logging.
>> [ERROR]
>> [ERROR] For more information about the errors and possible solutions,
>>
>> please read the following articles:
>> [ERROR] [Help 1]
>> http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
>>
>> any ideas?
>
> No ideas at all :-(
> Which version of maven do you use ?
>
> Luc
>
>> Dennis
>>
>>
>> Luc Maisonobe wrote:
>>> Le 24/06/2011 11:33, Dennis Hendriks a écrit :
>>>> > 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.
>>> The checkstyle file used is not the one from Apache Commons Math.
>> I'm
>>> not sure using the "rc" profile in the -Prc flag really cleaned
>>> everything on the first command, so the second command "mvn
>>> checkstyle:checkstyle" did use a cached value.
>>>
>>> Try something along these lines, without any -P flag:
>>>
>>> mvn clean site
>>>
>>> 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
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/
Posted by lu...@free.fr.
----- "Dennis Hendriks" <D....@tue.nl> a écrit :
> I tried 'mvn clean site', and got this:
>
> (...)
> main:
> [INFO] Executed tasks
> [INFO]
> [INFO] --- maven-remote-resources-plugin:1.0:process (default) @
> commons-math ---
> [INFO]
> [INFO] --- maven-resources-plugin:2.4.1:resources (default-resources)
> @
> commons-math ---
> [INFO] Using 'UTF-8' encoding to copy filtered resources.
> [INFO] Copying 2 resources to META-INF
> [INFO] Copying 1 resource to META-INF/localization
> [INFO]
> [INFO] --- maven-compiler-plugin:2.1:compile (default-compile) @
> commons-math ---
> [INFO] Nothing to compile - all classes are up to date
> [INFO]
> [INFO] <<< jdepend-maven-plugin:2.0-beta-2:generate (report:generate)
> @
> commons-math <<<
> [INFO] configuring report plugin
> org.codehaus.mojo:rat-maven-plugin:1.0-alpha-3
> [INFO] configuring report plugin
> org.codehaus.mojo:findbugs-maven-plugin:2.1
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Total time: 1:05.413s
> [INFO] Finished at: Mon Jun 27 08:54:24 CEST 2011
> [INFO] Final Memory: 32M/280M
> [INFO]
> ------------------------------------------------------------------------
> [ERROR] Failed to execute goal
> org.apache.maven.plugins:maven-site-plugin:3.0-beta-3:site
> (default-site)
> on project commons-math: failed to get Reports: Unable to parse
> configuration of mojo
> org.codehaus.mojo:findbugs-maven-plugin:2.1:findbugs
> for parameter localRepository: Abstract class or interface
> 'org.apache.maven.artifact.repository.DefaultArtifactRepository'
> cannot be
> instantiated -> [Help 1]
> [ERROR]
> [ERROR] To see the full stack trace of the errors, re-run Maven with
> the -e
> switch.
> [ERROR] Re-run Maven using the -X switch to enable full debug
> logging.
> [ERROR]
> [ERROR] For more information about the errors and possible solutions,
>
> please read the following articles:
> [ERROR] [Help 1]
> http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
>
> any ideas?
No ideas at all :-(
Which version of maven do you use ?
Luc
>
> Dennis
>
>
> Luc Maisonobe wrote:
> > Le 24/06/2011 11:33, Dennis Hendriks a écrit :
> >> > 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.
> >
> > The checkstyle file used is not the one from Apache Commons Math.
> I'm
> > not sure using the "rc" profile in the -Prc flag really cleaned
> > everything on the first command, so the second command "mvn
> > checkstyle:checkstyle" did use a cached value.
> >
> > Try something along these lines, without any -P flag:
> >
> > mvn clean site
> >
> > 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
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/
Posted by Dennis Hendriks <D....@tue.nl>.
I tried 'mvn clean site', and got this:
(...)
main:
[INFO] Executed tasks
[INFO]
[INFO] --- maven-remote-resources-plugin:1.0:process (default) @
commons-math ---
[INFO]
[INFO] --- maven-resources-plugin:2.4.1:resources (default-resources) @
commons-math ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources to META-INF
[INFO] Copying 1 resource to META-INF/localization
[INFO]
[INFO] --- maven-compiler-plugin:2.1:compile (default-compile) @
commons-math ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] <<< jdepend-maven-plugin:2.0-beta-2:generate (report:generate) @
commons-math <<<
[INFO] configuring report plugin org.codehaus.mojo:rat-maven-plugin:1.0-alpha-3
[INFO] configuring report plugin org.codehaus.mojo:findbugs-maven-plugin:2.1
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1:05.413s
[INFO] Finished at: Mon Jun 27 08:54:24 CEST 2011
[INFO] Final Memory: 32M/280M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-site-plugin:3.0-beta-3:site (default-site)
on project commons-math: failed to get Reports: Unable to parse
configuration of mojo org.codehaus.mojo:findbugs-maven-plugin:2.1:findbugs
for parameter localRepository: Abstract class or interface
'org.apache.maven.artifact.repository.DefaultArtifactRepository' cannot be
instantiated -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e
switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions,
please read the following articles:
[ERROR] [Help 1]
http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
any ideas?
Dennis
Luc Maisonobe wrote:
> Le 24/06/2011 11:33, Dennis Hendriks a écrit :
>> > 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.
>
> The checkstyle file used is not the one from Apache Commons Math. I'm
> not sure using the "rc" profile in the -Prc flag really cleaned
> everything on the first command, so the second command "mvn
> checkstyle:checkstyle" did use a cached value.
>
> Try something along these lines, without any -P flag:
>
> mvn clean site
>
> 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
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/
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 24/06/2011 11:33, Dennis Hendriks a écrit :
> > 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.
The checkstyle file used is not the one from Apache Commons Math. I'm
not sure using the "rc" profile in the -Prc flag really cleaned
everything on the first command, so the second command "mvn
checkstyle:checkstyle" did use a cached value.
Try something along these lines, without any -P flag:
mvn clean site
Luc
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
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/
Posted by Dennis Hendriks <D....@tue.nl>.
> 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
>
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/
Posted by Luc Maisonobe <Lu...@free.fr>.
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
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/
Posted by Dennis Hendriks <D....@tue.nl>.
> 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-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
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/
Posted by Dennis Hendriks <D....@tue.nl>.
> I prefer to have _one_ rule for code formatting.
> [Also, you nvere know when you might need an additional statement wihtin
> the conditional branch (e.g. when debugging).]
I would indeed consider that a reasonable reason to indeed force the braces.
Dennis
Gilles Sadowski wrote:
> Hi.
>
>>> 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 prefer to have _one_ rule for code formatting.
> [Also, you nvere know when you might need an additional statement wihtin the
> conditional branch (e.g. when debugging).]
>
>>> 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.
>
> +1 for enabling the rule.
>
>> [...]
>
> Best regards,
> Gilles
>
> ---------------------------------------------------------------------
> 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
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/
Posted by Phil Steitz <ph...@gmail.com>.
On 6/24/11 2:13 AM, Gilles Sadowski wrote:
> Hi.
>
>>> 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 prefer to have _one_ rule for code formatting.
> [Also, you nvere know when you might need an additional statement wihtin the
> conditional branch (e.g. when debugging).]
+1
Phil
>>> 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.
> +1 for enabling the rule.
>
>> [...]
> Best regards,
> Gilles
>
> ---------------------------------------------------------------------
> 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
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/
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
> >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 prefer to have _one_ rule for code formatting.
[Also, you nvere know when you might need an additional statement wihtin the
conditional branch (e.g. when debugging).]
> >
> >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.
+1 for enabling the rule.
> [...]
Best regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
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/
Posted by Luc Maisonobe <Lu...@free.fr>.
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
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/
Posted by Dennis Hendriks <D....@tue.nl>.
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...
I think the rule is already active in Checkstyle. I think I just chose to
ignore it here.
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?
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