[
https://issues.apache.org/jira/browse/HBASE-2618?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12876703#action_12876703
]
HBase Review Board commented on HBASE-2618:
-------------------------------------------
Message from: "Jonathan Gray" <[email protected]>
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/132/#review155
-----------------------------------------------------------
Ship it!
Looks good, Benoit. Only issue is in LocalHBaseCluster where default behavior
looks different now. I can fix that on commit, or just change that before
putting the patch up on JIRA?
trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java
<http://review.hbase.org/r/132/#comment785>
This seems to change behavior. Looks like it would previously default to
local if not set, now will default to non local?
trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/132/#comment786>
From this point forward in the diff, it looks like a majority of the
changes are whitespace. Not saying you need to throw it out but it's making
this long patch even longer :)
trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
<http://review.hbase.org/r/132/#comment787>
I guess there were a few tests that implemented HConstants but didn't
actually use anything in there? Maybe was before HBaseTestCase
- Jonathan
> Don't inherit from HConstants
> -----------------------------
>
> Key: HBASE-2618
> URL: https://issues.apache.org/jira/browse/HBASE-2618
> Project: HBase
> Issue Type: Wish
> Reporter: Benoit Sigoure
> Assignee: Benoit Sigoure
> Priority: Minor
>
> Can we stop using this idiom to inherit from HConstants? This is a known bad
> pattern and is recommended against in many places including Effective Java
> (item 17).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
