----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8370/#review14113 -----------------------------------------------------------
Good refactoring, easy +1 for me. /trunk/giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java <https://reviews.apache.org/r/8370/#comment30188> We should probably note that this only affects the default VertexResolver implementation. /trunk/giraph/src/main/java/org/apache/giraph/GraphStateConfigurable.java <https://reviews.apache.org/r/8370/#comment30179> I like this! Not sure about the Configurable bit though, since GraphState is not really configuration. What about GraphStateAware or something like that? /trunk/giraph/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java <https://reviews.apache.org/r/8370/#comment30187> Can't we provide this? /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexResolver.java <https://reviews.apache.org/r/8370/#comment30183> How about calling this DefaultVertexResolver and the interface just VertexResolver? /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexResolver.java <https://reviews.apache.org/r/8370/#comment30181> Nit: we seem to prefer the "vertices" plural form in Giraph. /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexResolver.java <https://reviews.apache.org/r/8370/#comment30182> Very clean. I would either remove the blank lines or interleave the algorithm description with each line. - Alessandro Presta On Dec. 6, 2012, 7:59 p.m., Nitay Joffe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8370/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2012, 7:59 p.m.) > > > Review request for giraph. > > > Description > ------- > > https://issues.apache.org/jira/browse/GIRAPH-444 > > > Diffs > ----- > > > /trunk/giraph-formats-contrib/src/main/java/org/apache/giraph/io/hcatalog/HCatalogEdgeInputFormat.java > 1417641 > /trunk/giraph/src/main/java/org/apache/giraph/GiraphConfiguration.java > 1417641 > /trunk/giraph/src/main/java/org/apache/giraph/GraphStateConfigurable.java > PRE-CREATION > > /trunk/giraph/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java > 1417641 > > /trunk/giraph/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java > 1417641 > > /trunk/giraph/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java > 1417641 > /trunk/giraph/src/main/java/org/apache/giraph/graph/VertexResolver.java > 1417641 > > Diff: https://reviews.apache.org/r/8370/diff/ > > > Testing > ------- > > > Thanks, > > Nitay Joffe > >
