> -----Original Message----- > From: Gilles [mailto:gil...@harfang.homelinux.org] > Sent: Friday, August 5, 2016 09:11 > To: Commons Developers List <dev@commons.apache.org> > Subject: Re: [5/7] [math] Fix "FastMath#round(..)" to comply to changed > contract of "Math#round()" in Java 8 > > Hi. > > Changes in this file seem unrelated to the commit message. > > Gilles [orcmid]
Yes, this appears to be a massive change involving what may largely be stylistic matters (if that is what Assert.assertMumble << assertMumble is all about). On first glance, it makes an excessive change that is extremely difficult to comprehend and review. What are the practices around expecting small focused single-purpose changes and, in this case, a small change that addresses the stated purpose of the commit with regard to the contract for FastMath.round() and its testing? Is this [VETO] territory, and would the committer be asked to revert the change and break it down? I ask this because I don't understand the Commons culture and practices in order to know. Oh, and is there a JIRA about this (the Math.round contract) or some other way to tie in whatever scholarship and traceability is involved in the specialized case of [MATH] software? (I may have missed this - sorry for asking if already answered.) - Dennis > > On Fri, 05 Aug 2016 15:06:53 -0000, ebo...@apache.org wrote: > > > > http://git-wip-us.apache.org/repos/asf/commons- > math/blob/83b70a37/src/test/java/org/apache/commons/math4/util/FastMathT > est.java > > > > ---------------------------------------------------------------------- > > diff --git > > a/src/test/java/org/apache/commons/math4/util/FastMathTest.java > > b/src/test/java/org/apache/commons/math4/util/FastMathTest.java > > index cc95c9c..1f94352 100644 > > --- a/src/test/java/org/apache/commons/math4/util/FastMathTest.java > > +++ b/src/test/java/org/apache/commons/math4/util/FastMathTest.java > > @@ -16,6 +16,10 @@ > > */ > > package org.apache.commons.math4.util; > > > > +import static org.junit.Assert.assertEquals; > > +import static org.junit.Assert.assertTrue; > > +import static org.junit.Assert.fail; > > + > > import java.lang.reflect.Method; > > import java.lang.reflect.Modifier; > > import java.lang.reflect.Type; > > @@ -30,8 +34,6 @@ import org.apache.commons.math4.dfp.DfpMath; > > import org.apache.commons.math4.exception.MathArithmeticException; > > import org.apache.commons.math4.rng.UniformRandomProvider; > > import org.apache.commons.math4.rng.RandomSource; > > -import org.apache.commons.math4.util.FastMath; > > -import org.apache.commons.math4.util.Precision; > > import org.junit.Assert; > > import org.junit.Before; > > import org.junit.Ignore; > > @@ -42,7 +44,6 @@ public class FastMathTest { > > private static final double MAX_ERROR_ULP = 0.51; > > private static final int NUMBER_OF_TRIALS = 1000; > > > > - > > private DfpField field; > > private UniformRandomProvider generator; > > > > @@ -67,22 +68,22 @@ public class FastMathTest { > > { Precision.SAFE_MIN, Precision.EPSILON } > > }; > > for (double[] pair : pairs) { > > - Assert.assertEquals("min(" + pair[0] + ", " + pair[1] + > > ")", > > - Math.min(pair[0], pair[1]), > > - FastMath.min(pair[0], pair[1]), > > - Precision.EPSILON); > > - Assert.assertEquals("min(" + pair[1] + ", " + pair[0] + > > ")", > > - Math.min(pair[1], pair[0]), > > - FastMath.min(pair[1], pair[0]), > > - Precision.EPSILON); > > - Assert.assertEquals("max(" + pair[0] + ", " + pair[1] + > > ")", > > - Math.max(pair[0], pair[1]), > > - FastMath.max(pair[0], pair[1]), > > - Precision.EPSILON); > > - Assert.assertEquals("max(" + pair[1] + ", " + pair[0] + > > ")", > > - Math.max(pair[1], pair[0]), > > - FastMath.max(pair[1], pair[0]), > > - Precision.EPSILON); > > + assertEquals("min(" + pair[0] + ", " + pair[1] + ")", > > + Math.min(pair[0], pair[1]), > > + FastMath.min(pair[0], pair[1]), > > + Precision.EPSILON); > > + assertEquals("min(" + pair[1] + ", " + pair[0] + ")", > > + Math.min(pair[1], pair[0]), > > + FastMath.min(pair[1], pair[0]), > > + Precision.EPSILON); > > + assertEquals("max(" + pair[0] + ", " + pair[1] + ")", > > + Math.max(pair[0], pair[1]), > > + FastMath.max(pair[0], pair[1]), > > + Precision.EPSILON); > > + assertEquals("max(" + pair[1] + ", " + pair[0] + ")", > > + Math.max(pair[1], pair[0]), > > + FastMath.max(pair[1], pair[0]), > > + Precision.EPSILON); > > } > > } > > > > @@ -100,39 +101,39 @@ public class FastMathTest { > > { Float.NaN, Float.POSITIVE_INFINITY } > > }; > > for (float[] pair : pairs) { > > - Assert.assertEquals("min(" + pair[0] + ", " + pair[1] + > > ")", > > - Math.min(pair[0], pair[1]), > > - FastMath.min(pair[0], pair[1]), > > - Precision.EPSILON); > > - Assert.assertEquals("min(" + pair[1] + ", " + pair[0] + > > ")", > > - Math.min(pair[1], pair[0]), > > - FastMath.min(pair[1], pair[0]), > > - Precision.EPSILON); > > - Assert.assertEquals("max(" + pair[0] + ", " + pair[1] + > > ")", > > - Math.max(pair[0], pair[1]), > > - FastMath.max(pair[0], pair[1]), > > - Precision.EPSILON); > > - Assert.assertEquals("max(" + pair[1] + ", " + pair[0] + > > ")", > > - Math.max(pair[1], pair[0]), > > - FastMath.max(pair[1], pair[0]), > > - Precision.EPSILON); > > + assertEquals("min(" + pair[0] + ", " + pair[1] + ")", > > + Math.min(pair[0], pair[1]), > > + FastMath.min(pair[0], pair[1]), > > + Precision.EPSILON); > > + assertEquals("min(" + pair[1] + ", " + pair[0] + ")", > > + Math.min(pair[1], pair[0]), > > + FastMath.min(pair[1], pair[0]), > > + Precision.EPSILON); > > + assertEquals("max(" + pair[0] + ", " + pair[1] + ")", > > + Math.max(pair[0], pair[1]), > > + FastMath.max(pair[0], pair[1]), > > + Precision.EPSILON); > > + assertEquals("max(" + pair[1] + ", " + pair[0] + ")", > > + Math.max(pair[1], pair[0]), > > + FastMath.max(pair[1], pair[0]), > > + Precision.EPSILON); > > } > > } > > > > @Test > > public void testConstants() { > > - Assert.assertEquals(Math.PI, FastMath.PI, 1.0e-20); > > - Assert.assertEquals(Math.E, FastMath.E, 1.0e-20); > > + assertEquals(Math.PI, FastMath.PI, 1.0e-20); > > + assertEquals(Math.E, FastMath.E, 1.0e-20); > > } > > > > > > <TRUNCATED> > > > --------------------------------------------------------------------- > 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
