narendly commented on a change in pull request #861: Make ClusterSetup
realm-aware
URL: https://github.com/apache/helix/pull/861#discussion_r389044068
##########
File path: helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
##########
@@ -1570,4 +1610,66 @@ public static void main(String[] args) throws Exception
{
int ret = processCommandLineArgs(args);
System.exit(ret);
}
+
+ public static class Builder {
Review comment:
@jiajunwang This was an option considered carefully, but let me note here
that having a parent builder doesn't actually remove duplicate code because all
setters and constructors will have to either be re-written or overriden. This
is because setters return Builder objects that aren't compatible with children
builders. Take a look at the ZkClusterVerifier PR - in that PR, it makes more
sense to have a parent Builder, but we don't see the reduction in duplicate
code.
In general, having an inheritance hierarchy for static Builders is not
necessarily a good idea because 1) it doesn't reduce code duplication, 2)
Builder's setter and validation logic need to / may evolve differently, and
inheritance might make that difficult in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
