pivotal-jbarrett commented on code in PR #954:
URL: https://github.com/apache/geode-native/pull/954#discussion_r846126196
##########
clicache/integration-test2/Cluster.cs:
##########
@@ -71,7 +73,7 @@ private bool StartLocators()
for (var i = 0; i < locatorCount_; i++)
{
var locator = new Locator(this, new List<Locator>(),
- name_ + "/locator/" + i.ToString(), i == 0);
+ name_ + "/locator/" + i.ToString(), i == 0, allowAttach_);
Review Comment:
This constructor is starting to get wild. What about a builder patter?
##########
clicache/integration-test2/Cluster.cs:
##########
@@ -200,6 +204,7 @@ public int Start()
.withMaxHeap("256m")
.withJmxManagerPort(cluster_.jmxManagerPort)
.withJmxManagerStart(startJmxManager_)
+ .withAllowAttach(allowAttach_, Address.address)
Review Comment:
What is behind the choice of the word "attach"? We are starting the debugger
agent, why not use a name that suggests that, like `withDebugAgent(...)`.
##########
clicache/integration-test2/Cluster.cs:
##########
@@ -71,7 +73,7 @@ private bool StartLocators()
for (var i = 0; i < locatorCount_; i++)
{
var locator = new Locator(this, new List<Locator>(),
Review Comment:
The second parameter here should be your `locators_` list so that each
locators is aware of the previously created.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
