> On Aug. 8, 2012, 8:15 p.m., Jakob Homan wrote: > > trunk/checkstyle.xml, line 264 > > <https://reviews.apache.org/r/6402/diff/2/?file=134575#file134575line264> > > > > I'm all for removing overly restrictive checkstyle requirements, but > > that should be done in a separate issue. Are these changes directly germane > > to this patch? > > Jaeho Shin wrote: > Yes. Otherwise, I would have to add same amount of code supporting > comments for suppressing RedundantThrows. I could separate this fix, and > only add lines without the deletion if you want. > > Eugene Koontz wrote: > I need the ConstantName CHECKSTYLE section that was removed here for a > separate reason (GIRAPH-211). It would be best to separate into a new JIRA or > perhaps several new JIRAs.
Actually I think just one JIRA would fine, with just Jaeho's entire checkstyle.xml improvement. Then we could discuss over on that JIRA, reviwing this change and hopefully keep it as-is as much as possible. I would file the JIRA but Apache Jira is down currently. :( - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6402/#review10034 ----------------------------------------------------------- On Aug. 6, 2012, 9:21 p.m., Jaeho Shin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6402/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2012, 9:21 p.m.) > > > Review request for giraph. > > > Description > ------- > > (copied from GIRAPH-277) > > The current way of implementing VertexInputFormat and VertexReader had bad > smell. It required users to understand how these two classes are glued > together, and forced similar codes to be duplicated in every new input > format. (Similarly for the VertexOutputFormat and VertexWriter.) Anyone who > wants to create a new format should create an underlying record reader or > writer at the right moment and delegate some calls to it, which seemed > unnecessary detail being exposed. Besides, type parameters had to appear all > over every new format code, which was extremely annoying for both reading > existing code and writing a new one. I was very frustrated writing my first > format code especially when I compared it to writing a new vertex code. I > thought writing a new input/output format should be as simple as vertex. > So, I have refactored TextVertexInputFormat and OutputFormat into new forms > that have no difference in their interfaces, but remove a lot of burden for > subclassing. Instead of providing static VertexReader base classes, I made it > a non-static inner-class of its format class, which helps eliminate the > repeated code for gluing these two, already tightly coupled classes. This has > additional advantage of eliminating all the Generics type variables on the > VertexReader side, which makes overall code much more concise. I added > several useful TextVertexReader base classes that can save efforts for > implementing line-oriented formats. > > > This addresses bug GIRAPH-277. > https://issues.apache.org/jira/browse/GIRAPH-277 > > > Diffs > ----- > > trunk/checkstyle.xml 1369914 > > trunk/src/main/java/org/apache/giraph/examples/IntIntNullIntTextInputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java > 1369914 > trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java > 1369914 > > trunk/src/main/java/org/apache/giraph/examples/SimpleTextVertexOutputFormat.java > 1369914 > > trunk/src/main/java/org/apache/giraph/examples/VertexWithComponentTextOutputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/graph/VertexWriter.java 1369914 > > trunk/src/main/java/org/apache/giraph/lib/AdjacencyListTextVertexInputFormat.java > PRE-CREATION > > trunk/src/main/java/org/apache/giraph/lib/AdjacencyListTextVertexOutputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/AdjacencyListVertexReader.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/IdWithValueTextOutputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexInputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/JsonBase64VertexOutputFormat.java > 1369914 > > trunk/src/main/java/org/apache/giraph/lib/JsonLongDoubleFloatDoubleVertexInputFormat.java > 1369914 > > trunk/src/main/java/org/apache/giraph/lib/JsonLongDoubleFloatDoubleVertexOutputFormat.java > 1369914 > > trunk/src/main/java/org/apache/giraph/lib/LongDoubleDoubleAdjacencyListVertexInputFormat.java > 1369914 > > trunk/src/main/java/org/apache/giraph/lib/TextDoubleDoubleAdjacencyListVertexInputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/TextVertexInputFormat.java > 1369914 > trunk/src/main/java/org/apache/giraph/lib/TextVertexOutputFormat.java > 1369914 > > trunk/src/test/java/org/apache/giraph/lib/TestAdjacencyListTextVertexOutputFormat.java > 1369914 > > trunk/src/test/java/org/apache/giraph/lib/TestIdWithValueTextOutputFormat.java > 1369914 > > trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java > 1369914 > > trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java > 1369914 > > Diff: https://reviews.apache.org/r/6402/diff/ > > > Testing > ------- > > > Thanks, > > Jaeho Shin > >
