pdxcodemonkey commented on a change in pull request #936:
URL: https://github.com/apache/geode-native/pull/936#discussion_r816872113



##########
File path: clicache/integration-test2/Cluster.cs
##########
@@ -70,8 +70,9 @@ private bool StartLocators()
 
             for (var i = 0; i < locatorCount_; i++)
             {
+                var startJmxManager = (i == 0);

Review comment:
       Okay, a few problems here:
   i) I'd really like to do this without introducing a boolean parameter, 
though I understand it may be unavoidable.
   ii) We don't need a new startJmxManager here, just pass i == 0 as the last 
parameter to the locator ctor
   iii) This was here before you got to it, but this loop is still broken for 
multiple locators, because the success variable is broken.  If you start two, 
and the first one fails but the second one works, you've buried an error.  
Please break out of the for loop if success is false.




-- 
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: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to