hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Gray <jg...@facebook.com>
Subject Review Request: HBASE-2618 Don't inherit from HConstants.
Date Sun, 06 Jun 2010 18:23:23 GMT
> On 2010-06-06 09:08:19, Jonathan Gray wrote:
> > This patch changes a bunch of tabbing patterns to be different from what is currently
done in the codebase.  Primarily the full indenting of arguments to align with each other.
> > 
> > As far as I know, this format is not used anywhere else in HBase.  I don't think
patches to clean up HConstants inheriting should do a partial change to a different style
for something unrelated?
> 
> Benoit Sigoure wrote:
>     As far as I can tell HBase doesn't seem to follow any consistent coding style, so
I did what everyone else seems to be doing, I let my editor do the re-indenting the way I
typically do.
>     I'll upload a new patch with the indentation change.
>     
>     Other than that, what do you think about the change?

It is mostly consistent.  In general the practice is to keep the same styling as the file
you're in or if a new file, in whatever else is like it that's already there.

Other than that, I'm a big +1 on removing the extending interface pattern we use on HConstants
and your patch looks good to me on that front.


- Jonathan


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


On 2010-06-05 23:47:11, Benoit Sigoure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/132/
> -----------------------------------------------------------
> 
> (Updated 2010-06-05 23:47:11)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2618 Don't inherit from HConstants.
> 
> Bonus: minor aesthetic / coding style clean ups and minor code changes.
> 
> 
> This addresses bug HBASE-2618.
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/HMerge.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/BaseScanner.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/RegionServerOperation.java 951516

>   trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/master/TableOperation.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java 951516

>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 951516

>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/HQuorumPeer.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKServerTool.java 951516 
>   trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 951516

>   trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java 951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestExplicitColumnTracker.java
951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetDeleteTracker.java
951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeyValueHeap.java 951516

>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java 951516

>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanDeleteTracker.java
951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestWildcardColumnTracker.java
951516 
>   trunk/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java 951516 
> 
> Diff: http://review.hbase.org/r/132/diff
> 
> 
> Testing
> -------
> 
> Code compiles.
> Tests are acting up on my machine right now (many of them fail with a weird message [1]
and Todd says he's been seeing similar failures for some time already, so I guess I'll try
to run them again next week when the New Moon arrives).
> 
> 
> [1] A number of tests fail with:
> org.apache.hadoop.hbase.client.NoServerForRegionException: Timed out trying to locate
root region because: Failed setting up proxy to /192.168.0.7:63773 after attempts=1
>         at org.apache.hadoop.hbase.client.HConnectionManager$TableServers.locateRootRegion(HConnectionManager.java:1031)
> Where, of course, 192.168.0.7 is my IP address.
> Some of the tests that are acting up:
> org.apache.hadoop.hbase.TestZooKeeper, org.apache.hadoop.hbase.regionserver.wal.TestLogRolling,
org.apache.hadoop.hbase.rest.TestScannersWithFilters, org.apache.hadoop.hbase.master.TestMasterWrongRS,
org.apache.hadoop.hbase.thrift.TestThriftServer, org.apache.hadoop.hbase.master.TestMasterTransitions,
org.apache.hadoop.hbase.rest.TestStatusResource, org.apache.hadoop.hbase.client.TestFromClientSide,
org.apache.hadoop.hbase.TestMultiParallelPut, org.apache.hadoop.hbase.master.TestRegionManager,
org.apache.hadoop.hbase.mapreduce.TestTimeRangeMapRed
> 
> 
> Thanks,
> 
> Benoit
> 
>

Mime
View raw message