[ 
https://issues.apache.org/jira/browse/HBASE-21071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16585018#comment-16585018
 ] 

stack commented on HBASE-21071:
-------------------------------

Man. This stuff is a mess. Your patch helps a load.

I saw 'create' as a builder method and didn't know what it was for. Your 
builder methods could do with a bit of javadoc if you don't mind. You might 
also take the opportunity to fix our mess... the create method should be 
createRootDir or some such.... if it was called that, you might even be able to 
do w/o out javadoc. The method name alone would do.

No need of the final here... 

final StartMiniClusterOption option

... it does nothing IIRC.

This is a good change:

    MiniHBaseCluster hbm = htu.startMiniHBaseCluster(1, 1);     104         
final MiniHBaseCluster hbm = htu.startMiniHBaseCluster();

This stuff below is great too...

StartMiniClusterOption.builder()
72              .masterClass(TestMasterMetrics.MyMaster.class).build();
73          TEST_UTIL.startMiniCluster(option);

Is withWALDir(true) same as create?

Patch looks great so far.

Agree, that leaving startMiniCluster(int numMasters, int numSlaves) in place as 
a short cut especially given it is used in so many places.

Nice cleanup.

> HBaseTestingUtility::startMiniCluster() to use builder pattern
> --------------------------------------------------------------
>
>                 Key: HBASE-21071
>                 URL: https://issues.apache.org/jira/browse/HBASE-21071
>             Project: HBase
>          Issue Type: Bug
>          Components: test
>    Affects Versions: 3.0.0
>            Reporter: Mingliang Liu
>            Assignee: Mingliang Liu
>            Priority: Major
>         Attachments: HBASE-21071.000.patch, HBASE-21071.001.patch
>
>
> Currently there are 13 {{startMiniCluster()}} methods to set up a mini 
> cluster. I'm not surprised if we have a few more in future. It's good to 
> support different combination of optional parameters. We have to pick up one 
> of them carefully while still wondering the default values of other 
> parameters; if we add a new option, we may bring more new methods.
> One solution is to use builder pattern: create a class {{MiniClusterOptions}} 
> along with a static class {{MiniClusterOptionsBuilder}}, create a new method  
> {{startMiniCluster(MiniClusterOptions)}}. In {{master}} we delete the old 13 
> methods while in branch-2, we deprecate the old 13 methods.
> Thoughts?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to