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