-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/128/#review137
-----------------------------------------------------------


This looks great Hammer.  No major comments.


trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment721>

    No.  Setting a timestamp actually just sets the timerange with (stamp, 
stamp+1)



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment722>

    I've seen this in other places like HMaster where sometimes we initialize 
through command-line and other times programatically within something else, 
like in tests or to re-initialize... I think



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java
<http://review.hbase.org/r/128/#comment723>

    This is one thing that is fairly mixed in the codebase.  Not sure which is 
the right way, I like things spaced out but I might be in the minority.



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment724>

    This is unfortunately mixed a bit... I think the client APIs say TimeStamp, 
not sure why exactly but I guess just align with that?



trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java
<http://review.hbase.org/r/128/#comment725>

    The form I believe we follow is that inputs can be either null or 0-length 
array, but that data returned is a 0-length array.  Would need to verify this 
is always the case but I think it is.


- Jonathan


On 2010-06-05 23:16:10, Jeff Hammerbacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/128/
> -----------------------------------------------------------
> 
> (Updated 2010-06-05 23:16:10)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Initial patch; some javadoc and tests missing, but I wanted to get some 
> initial feedback on the approach. My apologies for sticking a patch on the 
> JIRA before the review. I should have read further on the HowToContribute 
> JIRA.
> 
> 
> This addresses bug HBASE-2400.
> 
> 
> Diffs
> -----
> 
>   trunk/bin/hbase 951826 
>   trunk/pom.xml 951826 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AAlreadyExists.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AClusterStatus.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumn.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnFamilyDescriptor.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnValue.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ACompressionAlgorithm.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ADelete.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AFamilyDescriptor.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AGet.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIOError.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIllegalArgument.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AMasterNotRunning.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/APut.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ARegionLoad.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResult.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResultEntry.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AScan.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerAddress.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerInfo.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerLoad.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableDescriptor.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableExists.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATimeRange.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/HBase.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/IOError.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/TCell.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.avpr PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/package.html PRE-CREATION 
>   trunk/src/test/java/org/apache/hadoop/hbase/avro/TestAvroServer.java 
> PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/128/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jeff
> 
>

Reply via email to