> 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?
> 
> Jonathan Gray wrote:
>     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.

As far as the tests go, I've never actually seen that failing pattern.  I've 
been running unit tests on trunk in at least 4 different environments.

I wonder where this is coming from... Is there a jira open for this specific 
failure?  If not, let's open one so we can get to the bottom of it.


- 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
> 
>

Reply via email to