Agree, which is why I'd rather just nail this once rather than dribble it in.
It's reasonable to say you just don't think the formatting and style stuff matters, but I find it does, indirectly, from a "broken windows policy" perspective. If your code *looks* a bit uneven and inconsistent, people have less compunction about checking in more uneven code (since it already is) and have a general sense that it's alright to check in stuff that's maybe not as completely baked as would be ideal -- since it looks like anything goes. And I don't think we're spending too *much* time thinking about design and such. So is it so painful to let someone else touch up, on everyone's behalf, the code base? we really should have been checking in better code from the get-go. Turned around... I also don't see the argument against cleaning this up. Do you have a patch? fine, we can wait until you're ready check in. This is a real no-skin-off-your-nose change so, ? On Sat, May 29, 2010 at 2:11 PM, Jake Mannix <[email protected]> wrote: > Erggg..... once I again I will state that I strongly prefer Grant's > approach. > > Do findbugs and formatting "errors" actually cause people physical pain? > > Why does this keep coming up? > > -jake > > ps. no I doubt anyone is working on those files. Doesn't change my > opinion of massive formatting checkins. > > On Sat, May 29, 2010 at 10:58 AM, Robin Anil <[email protected]> wrote: > >> Correct me If I am wrong, I believe there are no conflicts for many of the >> following top violators (except the matrix/linalg which Jake may have some >> changes). So there shouldn't be a problem with formatting these. Is anyone >> working on any of these classes? >> >> Robin >> filename l m h number >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/math/Bessel.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/math/Bessel.java> >> 0 224 0 224 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/engine/RandomSeedTable.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/engine/RandomSeedTable.java> >> 0 220 0 220 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/math/Arithmetic.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/math/Arithmetic.java> >> 0 215 0 215 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Probability.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Probability.java> >> 0 104 0 104 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Fun.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Fun.java> >> 0 88 0 88 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Beta.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Beta.java> >> 0 84 0 84 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/HyperGeometric.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/HyperGeometric.java> >> 0 82 0 82 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Property.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Property.java> >> 0 71 0 71 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix2D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix2D.java> >> 0 68 0 68 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/stat/quantile/QuantileFinderFactory.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/stat/quantile/QuantileFinderFactory.java> >> 0 57 0 57 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Gamma.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Gamma.java> >> 0 54 0 54 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Transform.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Transform.java> >> 0 51 0 51 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Algebra.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Algebra.java> >> 0 48 0 48 >> >> trunk/math/src/main/java/org/apache/mahout/math/function/Functions.java<file/trunk/math/src/main/java/org/apache/mahout/math/function/Functions.java> >> 0 47 0 47 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Poisson.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Poisson.java> >> 0 47 0 47 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Binomial.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Binomial.java> >> 0 43 0 43 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/stat/quantile/QuantileCalc.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/stat/quantile/QuantileCalc.java> >> 0 40 0 40 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/sampling/RandomSampler.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/sampling/RandomSampler.java> >> 0 38 0 38 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Formatter.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Formatter.java> >> 0 38 0 38 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix3D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix3D.java> >> 0 38 0 38 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/engine/MersenneTwister.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/engine/MersenneTwister.java> >> 0 28 0 28 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/LUDecompositionQuick.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/LUDecompositionQuick.java> >> 0 26 0 26 >> trunk/math/src/main/java/org/apache/mahout/math/Partitioning.java<file/trunk/math/src/main/java/org/apache/mahout/math/Partitioning.java>025025 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/EigenvalueDecomposition.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/EigenvalueDecomposition.java> >> 0 24 0 24 >> >> trunk/math/src/main/java/org/apache/mahout/math/decomposer/hebbian/HebbianSolver.java<file/trunk/math/src/main/java/org/apache/mahout/math/decomposer/hebbian/HebbianSolver.java> >> 0 23 0 23 >> >> trunk/math/src/main/java/org/apache/mahout/math/AbstractVector.java<file/trunk/math/src/main/java/org/apache/mahout/math/AbstractVector.java> >> 0 21 0 21 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Distributions.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Distributions.java> >> 0 21 0 21 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/DoubleFactory2D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/DoubleFactory2D.java> >> 0 21 0 21 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Descriptive.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/stat/Descriptive.java> >> 0 20 0 20 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Statistic.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/doublealgo/Statistic.java> >> 0 20 0 20 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/SeqBlas.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/SeqBlas.java> >> 0 19 0 19 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/SingularValueDecomposition.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/SingularValueDecomposition.java> >> 0 19 0 19 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/math/IntFunctions.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/math/IntFunctions.java> >> 0 18 0 18 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/AbstractMatrix3D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/AbstractMatrix3D.java> >> 0 18 0 18 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/QRDecomposition.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/QRDecomposition.java> >> 0 18 0 18 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Gamma.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Gamma.java> >> 0 16 0 16 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/TridiagonalDoubleMatrix2D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/TridiagonalDoubleMatrix2D.java> >> 0 16 0 16 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/Hyperbolic.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/Hyperbolic.java> >> 0 15 0 15 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Blas.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/Blas.java> >> 0 15 0 15 >> >> trunk/math/src/main/java/org/apache/mahout/math/jet/random/PoissonSlow.java<file/trunk/math/src/main/java/org/apache/mahout/math/jet/random/PoissonSlow.java> >> 0 14 0 14 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix1D.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/impl/DenseDoubleMatrix1D.java> >> 0 13 0 13 >> >> trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/CholeskyDecomposition.java<file/trunk/math/src/main/java/org/apache/mahout/math/matrix/linalg/CholeskyDecomposition.java> >> 0 13 0 13 >> On Sat, May 29, 2010 at 11:20 PM, Benson Margulies <[email protected]> >> wrote: >> > There are arguments in both directions. In my view, the ideal is: >> > >> > 1) declare a target date. >> > 2) everyone clears the deck of patches. >> > 3) Reformat >> > >> > Grant's proposal, which goes >> > >> > 1) have a reason to modify some particular bit >> > 2) check in patch >> > 3) check in reformat before someone else starts a patch >> > >> > is not bad, either. >> > >> > >> > On Sat, May 29, 2010 at 1:30 PM, Grant Ingersoll <[email protected] >> >wrote: >> > >> >> >> >> On May 29, 2010, at 11:11 AM, Robin Anil wrote: >> >> >> >> > Math module clearly doesn't conform to the style guidelines. Does it >> >> > make sense to go and clean it entirely or should we do it for the ones >> >> > we use, when we use it? >> >> > >> >> > >> >> >> >> I'm not a big fan of massive formatting changes. It breaks a lot of >> >> otherwise good patches. I usually apply them right as I'm about to >> commit >> >> on the files I have open. >> >> >> >> -Grant >> > >> >
