Review Board 1.7.22


GIRAPH-336

Review Request #7310 - Created Sept. 27, 2012 and updated

Avery Ching
Reviewers
giraph
giraph
Review of GIRAPH-336.patch for Brian.

FYI, this fails to patch cleanly for me.

aching@achingmbp15:~/git/git_svn_giraph_trunk$ git status
# On branch GIRAPH-328
nothing to commit (working directory clean)
aching@achingmbp15:~/git/git_svn_giraph_trunk$ patch -p0 < ~/Desktop/GIRAPH-336.patch 
patching file giraph-formats-contrib/src/hcatalog/java/org/apache/giraph/format/hcatalog/HiveGiraphRunner.java
Hunk #1 FAILED at 45.
1 out of 1 hunk FAILED -- saving rejects to file giraph-formats-contrib/src/hcatalog/java/org/apache/giraph/format/hcatalog/HiveGiraphRunner.java.rej
patching file giraph-formats-contrib/src/hcatalog/java/org/apache/giraph/format/hcatalog/HCatalogVertexInputFormat.java
patching file giraph-formats-contrib/src/hcatalog/java/org/apache/giraph/format/hcatalog/HCatalogVertexOutputFormat.java
patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexInputFormat.java
patching file giraph-formats-contrib/src/main/java/org/apache/giraph/format/accumulo/AccumuloVertexOutputFormat.java
patching file giraph-formats-contrib/pom.xml

 
Total:
11
Open:
11
Resolved:
0
Dropped:
0
Status:
From:
Description From Last Updated Status
Do we need this? How did it build before? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Would be nice to align these comments with the variable indent. This happens a lot in this code. Avery Ching Sept. 27, 2012, 5:52 a.m. Open
shouldn't this be static (and capitalized)? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
These should be converted to using log4j, not just removed. Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Please keep with log4j Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Convert to using logj4? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Why remove? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Why remove? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Why remove? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Why remove? Avery Ching Sept. 27, 2012, 5:52 a.m. Open
Please remove these. They aren't needed since they are described in the parent class javadoc. Avery Ching Sept. 27, 2012, 5:55 a.m. Open
Posted (Sept. 27, 2012, 5:52 a.m.)

   

  
http://svn.apache.org/repos/asf/giraph/trunk/giraph-formats-contrib/pom.xml (Diff revision 1)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
Do we need this?  How did it build before?
  1. This should already be in the codebase. You just need to compile it with 'mvn -Phcatalog clean verify'. This fixes that: https://issues.apache.org/jira/browse/GIRAPH-343. Patch is available ;)
Would be nice to align these comments with the variable indent.  This happens a lot in this code.
shouldn't this be static (and capitalized)?
These should be converted to using log4j, not just removed.
Please keep with log4j
Convert to using logj4?
Why remove?
Why remove?
Why remove?
Why remove?
Posted (Sept. 27, 2012, 5:55 a.m.)
One more thing.
Please remove these.  They aren't needed since they are described in the parent class javadoc.