Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread Gilles Sadowski 
Hi.

 Gilles,
 
 This should have been done in three separate commits.
 
 The first one, at least, should have been separated.  It is easier
 for reviewers and makes the commit log clearer if we separate
 formatting / javadoc cleanup commits from those that update or
 change the code.  The JIRA reference will pull all of these diffs
 under the referenced issue.  It is better if the commits that
 reference the issue are directly related to the issue.

Yes, it would be better. But I find it not so bad, as a compromise between
reviewers's (potential) work and my (actual) work. When the change brought
by the issue (changing the exceptions) implies modifications in the unit
tests, and I see that they are written in pre-Junit4 format, I do both types
of changes. It is pretty obvious which is which. Ditto for Javadoc: the
exception is referenced in the Javadoc, and when I spot something else to be
changed too, I do it.

For such trivial changes, I simply don't want to do it in several passes.

I do the work (MATH-423) incrementally. Now, if other people would
participate to that task and all unit tests would be upgraded, that would
solve your problem. [This issue is a trivial task that was opened more than
five months ago.]


Regards,
Gilles

 
 Thanks,
 
 Phil
  Modified:
  
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
  
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Erf.java
  
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Gamma.java
  
  commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/BetaTest.java
  
  commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/ErfTest.java
  
  commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/GammaTest.java
 
  Modified: 
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
  URL: 
  http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java?rev=1083323r1=1083322r2=1083323view=diff
  ==
  --- 
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
   (original)
  +++ 
  commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
   Sat Mar 19 22:49:59 2011
  @@ -16,7 +16,6 @@
*/
   package org.apache.commons.math.special;
   
  -import org.apache.commons.math.MathException;
   import org.apache.commons.math.util.ContinuedFraction;
   import org.apache.commons.math.util.FastMath;
   
  @@ -27,31 +26,28 @@ import org.apache.commons.math.util.Fast
* @version $Revision$ $Date$
*/
   public class Beta {
  -
   /** Maximum allowed numerical error. */
   private static final double DEFAULT_EPSILON = 10e-15;
   
   /**
* Default constructor.  Prohibit instantiation.
*/
  -private Beta() {
  -super();
  -}
  +private Beta() {}
   
   /**
* Returns the
* a href=http://mathworld.wolfram.com/RegularizedBetaFunction.html;
* regularized beta function/a I(x, a, b).
*
  - * @param x the value.
  - * @param a the a parameter.
  - * @param b the b parameter.
  - * @return the regularized beta function I(x, a, b)
  + * @param x Value.
  + * @param a Parameter {@code a}.
  + * @param b Parameter {@code b}.
  + * @return the regularized beta function I(x, a, b).
  + * @throws org.apache.commons.math.exception.MaxCountExceededException
  + * if the algorithm fails to converge.
* @throws MathException if the algorithm fails to converge.
*/
  -public static double regularizedBeta(double x, double a, double b)
  -throws MathException
  -{
  +public static double regularizedBeta(double x, double a, double b) {
   return regularizedBeta(x, a, b, DEFAULT_EPSILON, 
  Integer.MAX_VALUE);
   }
   
  @@ -60,18 +56,19 @@ public class Beta {
* a href=http://mathworld.wolfram.com/RegularizedBetaFunction.html;
* regularized beta function/a I(x, a, b).
*
  - * @param x the value.
  - * @param a the a parameter.
  - * @param b the b parameter.
  + * @param x Value.
  + * @param a Parameter {@code a}.
  + * @param b Parameter {@code b}.
* @param epsilon When the absolute value of the nth item in the
  - *series is less than epsilon the approximation ceases
  - *to calculate further elements in the series.
  + * series is less than epsilon the approximation ceases to calculate
  + * further elements in the series.
* @return the regularized beta function I(x, a, b)
  - * @throws MathException if the algorithm fails to converge.
  + * @throws org.apache.commons.math.exception.MaxCountExceededException
  + * if the algorithm fails to

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread sebb 
On 20 March 2011 10:12, Gilles Sadowski gil...@harfang.homelinux.org wrote:
 Hi.

 Gilles,

 This should have been done in three separate commits.

 The first one, at least, should have been separated.  It is easier
 for reviewers and makes the commit log clearer if we separate
 formatting / javadoc cleanup commits from those that update or
 change the code.  The JIRA reference will pull all of these diffs
 under the referenced issue.  It is better if the commits that
 reference the issue are directly related to the issue.

+1

 Yes, it would be better. But I find it not so bad, as a compromise between
 reviewers's (potential) work and my (actual) work.

However there are generally several reviewers, each of whom will have
to do extra work.

And it makes it much easier for future maintainers when looking at past history.

Or indeed if one aspect of the commit has to be reverted.

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread Gilles Sadowski 
On Sun, Mar 20, 2011 at 01:30:37PM +, sebb wrote:
 On 20 March 2011 10:12, Gilles Sadowski gil...@harfang.homelinux.org wrote:
  Hi.
 
  Gilles,
 
  This should have been done in three separate commits.
 
  The first one, at least, should have been separated.  It is easier
  for reviewers and makes the commit log clearer if we separate
  formatting / javadoc cleanup commits from those that update or
  change the code.  The JIRA reference will pull all of these diffs
  under the referenced issue.  It is better if the commits that
  reference the issue are directly related to the issue.
 
 +1
 
  Yes, it would be better. But I find it not so bad, as a compromise between
  reviewers's (potential) work and my (actual) work.
 
 However there are generally several reviewers, each of whom will have
 to do extra work.
 
 And it makes it much easier for future maintainers when looking at past 
 history.
 
 Or indeed if one aspect of the commit has to be reverted.

I know all of these arguments; thank you for repeating them. I've also
repeated several times that it is not up to other people to decide that I
should spend x minutes more on an issue because they want to double-check
each line which I modify. The more so if the changes are so trivial (like
that issue + Javadoc + Junit 4) that it will be quite unlikely they would
need to be reverted. [Is that the case, here? Shall a revert be needed in
case I introduced a typo in the Javadoc comments? I don't think so.]

I seriously doubt anyone here now or in the future will spend his time
looking at several months old diffs...  If he would, he could as well deal
with the issue that raises such useless discussions.


Regards,
Gilles

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread Phil Steitz 
On 3/20/11 10:08 AM, Gilles Sadowski wrote:
 On Sun, Mar 20, 2011 at 01:30:37PM +, sebb wrote:
 On 20 March 2011 10:12, Gilles Sadowski gil...@harfang.homelinux.org wrote:
 Hi.

 Gilles,

 This should have been done in three separate commits.

 The first one, at least, should have been separated.  It is easier
 for reviewers and makes the commit log clearer if we separate
 formatting / javadoc cleanup commits from those that update or
 change the code.  The JIRA reference will pull all of these diffs
 under the referenced issue.  It is better if the commits that
 reference the issue are directly related to the issue.
 +1

 Yes, it would be better. But I find it not so bad, as a compromise between
 reviewers's (potential) work and my (actual) work.
 However there are generally several reviewers, each of whom will have
 to do extra work.

 And it makes it much easier for future maintainers when looking at past 
 history.

 Or indeed if one aspect of the commit has to be reverted.
 I know all of these arguments; thank you for repeating them. I've also
 repeated several times that it is not up to other people to decide that I
 should spend x minutes more on an issue because they want to double-check
 each line which I modify.
Gilles,

Sorry to be blunt here, but it *is* up to the community to decide
how we manage our source code.  We have established practices that
we expect all committers to abide by.  These practices include
separating commits.  You need to do that.  Thanks.

Phil
  The more so if the changes are so trivial (like
 that issue + Javadoc + Junit 4) that it will be quite unlikely they would
 need to be reverted. [Is that the case, here? Shall a revert be needed in
 case I introduced a typo in the Javadoc comments? I don't think so.]

 I seriously doubt anyone here now or in the future will spend his time
 looking at several months old diffs...  If he would, he could as well deal
 with the issue that raises such useless discussions.


 Regards,
 Gilles

 -
 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

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread Gilles Sadowski 
Phil,

 Sorry to be blunt here, but it *is* up to the community to decide
 how we manage our source code.  We have established practices that
 we expect all committers to abide by.  These practices include
 separating commits.  You need to do that.  Thanks.

Yes, you are blunt, and patronizing.
Some things which contribute more to the overall quality of the project, you
don't care about (referring, namely, to my suggestions about having nicer,
more uniform comments) but you are being picky every time I dare to step,
slightly, out of the community path (which, it should be noted, does not
happen very often, considering the number of commits I did).

If the community is not statisfied with my contributions to CM, and people
think that I am more a burden than a help for the project, then maybe I
should stop contributing.


Gilles

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-20 Thread Phil Steitz 
On 3/20/11 12:18 PM, Gilles Sadowski wrote:
 Phil,

 Sorry to be blunt here, but it *is* up to the community to decide
 how we manage our source code.  We have established practices that
 we expect all committers to abide by.  These practices include
 separating commits.  You need to do that.  Thanks.
 Yes, you are blunt, and patronizing.
 Some things which contribute more to the overall quality of the project, you
 don't care about (referring, namely, to my suggestions about having nicer,
 more uniform comments) but you are being picky every time I dare to step,
 slightly, out of the community path (which, it should be noted, does not
 happen very often, considering the number of commits I did).

Yes, that is correct.  At Apache, community comes first.  I don't
complain about javadoc changes that depart from Sun standards (which
we agreed to adopt earlier) and do not improve the code because I
don't consider that worth arguing about.  But I *do* complain about
practices that do not conform to community standards expected of
committers.

Phil

 If the community is not statisfied with my contributions to CM, and people
 think that I am more a burden than a help for the project, then maybe I
 should stop contributing.


 Gilles

 -
 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

Re: svn commit: r1083323 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/special/ test/java/org/apache/commons/math/special/

2011-03-19 Thread Phil Steitz 
On 3/19/11 3:49 PM, er...@apache.org wrote:
 Author: erans
 Date: Sat Mar 19 22:49:59 2011
 New Revision: 1083323

 URL: http://svn.apache.org/viewvc?rev=1083323view=rev
 Log:
 MATH-446
 Removed checked exceptions.
 Some Javadoc cleanup.
 Tests upgraded to Junit 4 (MATH-423).

Gilles,

This should have been done in three separate commits.

The first one, at least, should have been separated.  It is easier
for reviewers and makes the commit log clearer if we separate
formatting / javadoc cleanup commits from those that update or
change the code.  The JIRA reference will pull all of these diffs
under the referenced issue.  It is better if the commits that
reference the issue are directly related to the issue.

Thanks,

Phil
 Modified:
 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Erf.java
 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Gamma.java
 
 commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/BetaTest.java
 
 commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/ErfTest.java
 
 commons/proper/math/trunk/src/test/java/org/apache/commons/math/special/GammaTest.java

 Modified: 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
 URL: 
 http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java?rev=1083323r1=1083322r2=1083323view=diff
 ==
 --- 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
  (original)
 +++ 
 commons/proper/math/trunk/src/main/java/org/apache/commons/math/special/Beta.java
  Sat Mar 19 22:49:59 2011
 @@ -16,7 +16,6 @@
   */
  package org.apache.commons.math.special;
  
 -import org.apache.commons.math.MathException;
  import org.apache.commons.math.util.ContinuedFraction;
  import org.apache.commons.math.util.FastMath;
  
 @@ -27,31 +26,28 @@ import org.apache.commons.math.util.Fast
   * @version $Revision$ $Date$
   */
  public class Beta {
 -
  /** Maximum allowed numerical error. */
  private static final double DEFAULT_EPSILON = 10e-15;
  
  /**
   * Default constructor.  Prohibit instantiation.
   */
 -private Beta() {
 -super();
 -}
 +private Beta() {}
  
  /**
   * Returns the
   * a href=http://mathworld.wolfram.com/RegularizedBetaFunction.html;
   * regularized beta function/a I(x, a, b).
   *
 - * @param x the value.
 - * @param a the a parameter.
 - * @param b the b parameter.
 - * @return the regularized beta function I(x, a, b)
 + * @param x Value.
 + * @param a Parameter {@code a}.
 + * @param b Parameter {@code b}.
 + * @return the regularized beta function I(x, a, b).
 + * @throws org.apache.commons.math.exception.MaxCountExceededException
 + * if the algorithm fails to converge.
   * @throws MathException if the algorithm fails to converge.
   */
 -public static double regularizedBeta(double x, double a, double b)
 -throws MathException
 -{
 +public static double regularizedBeta(double x, double a, double b) {
  return regularizedBeta(x, a, b, DEFAULT_EPSILON, Integer.MAX_VALUE);
  }
  
 @@ -60,18 +56,19 @@ public class Beta {
   * a href=http://mathworld.wolfram.com/RegularizedBetaFunction.html;
   * regularized beta function/a I(x, a, b).
   *
 - * @param x the value.
 - * @param a the a parameter.
 - * @param b the b parameter.
 + * @param x Value.
 + * @param a Parameter {@code a}.
 + * @param b Parameter {@code b}.
   * @param epsilon When the absolute value of the nth item in the
 - *series is less than epsilon the approximation ceases
 - *to calculate further elements in the series.
 + * series is less than epsilon the approximation ceases to calculate
 + * further elements in the series.
   * @return the regularized beta function I(x, a, b)
 - * @throws MathException if the algorithm fails to converge.
 + * @throws org.apache.commons.math.exception.MaxCountExceededException
 + * if the algorithm fails to converge.
   */
 -public static double regularizedBeta(double x, double a, double b,
 -double epsilon) throws MathException
 -{
 +public static double regularizedBeta(double x,
 + double a, double b,
 + double epsilon) {
  return regularizedBeta(x, a, b, epsilon, Integer.MAX_VALUE);
  }
  
 @@ -79,15 +76,16 @@ public class Beta {
   * Returns the regularized beta function I(x, a, b).
   *
   * @param x the value.
 - * @param a the a parameter.
 - * @param b the b parameter.
 + * @param a