mahout-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robin Anil <robin.a...@gmail.com>
Subject Re: Mass Code Cleanup
Date Sun, 14 Feb 2010 19:53:36 GMT
Some stats on the current trunk v/s previous trunk

Checkstyle violations by package after code format

2664 math
 739 core
 547 collections
 149 examples
 115 utils
   9 taste-web

earlier

2664 math
2349 core
 938 examples
  547 collections
  238 utils
     9 taste-web


Current Checkstyle violators by algorithms

3024 math
 295 cf
 268 clustering
 217 collections
 131 df
 106 utils
  77 common
  41 ga
  37 fpm
  17 classifier
   9 text

Math is anyways mainly colt style formatting. Disregarding math and
collections Top Violating classes

  16
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/common/parameters/Parametered.java
  14
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/df/tools/FrequenciesJob.java
  14
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/DirichletDriver.java
  13
/Users/robinanil/mahout-clean/examples/src/main/java/org/apache/mahout/clustering/dirichlet/DisplayDirichlet.java
  10
/Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/vectors/lucene/ClusterLabels.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/df/mapreduce/MapredMapper.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/fuzzykmeans/SoftCluster.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/models/L1Model.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/DirichletCluster.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/model/jdbc/AbstractJDBCDataModel.java
  10
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/common/FastMap.java
   9
/Users/robinanil/mahout-clean/examples/src/main/java/org/apache/mahout/clustering/canopy/DisplayCanopy.java
   9
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/kmeans/Cluster.java
   9
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/model/GenericBooleanPrefDataModel.java
   8
/Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/clustering/ClusterDumper.java
   8
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/slopeone/jdbc/AbstractJDBCDiffStorage.java
   8
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/TreeClusteringRecommender2.java
   8
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/GenericItemBasedRecommender.java
   8
/Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/common/FastByIDMap.java
   7
/Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/vectors/lucene/CachedTermInfo.java










On Mon, Feb 15, 2010 at 1:02 AM, Robin Anil <robin.anil@gmail.com> wrote:

>
> On Mon, Feb 15, 2010 at 12:45 AM, Sean Owen <srowen@gmail.com> wrote:
>
>> I looked at the result and it seems a fair bit more happened. I don't
>> mind line rewrapping in javadoc, or putting newlines after <p>, though
>> I do tend to think those are noise changes not worth applying.
>>
>> I don't know if we ever talked about minor stuff like, do java.*
> imports go first or last. But we seemed to have had a convention
> going, and every file got reversed. That doesn't seem worthwhile.
>
> These are part of the Lucene Code formatter. Taken from the Lucene wiki.
> Mahout version just cleans up method definitions  but sticks to the lucene
> version.
>
>
>
>>
>> But why are all static references qualified now? like, all "log" lines
>> are now "MyClass.log(...)" That doesn't seem right to me at all. Lots
>> of code is less readable and running well over 120 columns.
>>
>> return 2.0 * (LogLikelihoodSimilarity.logL(k1 / n1, k1, n1)
>>                  + LogLikelihoodSimilarity.logL(k2 / n2, k2, n2) -
>> LogLikelihoodSimilarity.logL(p, k1, n1) - LogLikelihoodSimilarity
>>        .logL(p, k2, n2));
>>
>>
>
>> This. I am looking. Can be an easier fix than rolling back.
>>
>
>
>> I'd propose a rollback just for the last item -- I can't deal with
>> that one myself. Then briefly discuss the rest of the changes that
>> happened and whether there's consensus. Then hit it again. I do like
>> standardization.
>>
>  Take a look at the checkstyle too. If we make that as the
> target standardization, then there wont be any issue going forward. If
> checkstyle says ok, the code should be ok.
>
> On Sat, Feb 13, 2010 at 2:55 PM, Robin Anil <robin.anil@gmail.com> wrote:
>> > I just did a mass code cleanup.
>> >
>> > Mainly comprising of
>> > -Extra blank line removal
>> > -Organize Imports across all packages.
>> > -Making local variables final
>> >
>> > No reordering of methods or code style changes are applied.
>> >
>> > Any objections or any particular class to withhold from committing.
>> >
>> > Robin
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message