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