[GitHub] [geode] mhansonp commented on a change in pull request #6116: GEODE-8877: Configurable UDP membership address

[GitBox <gi...@apache.org>]

mhansonp commented on a change in pull request #6116:
URL: https://github.com/apache/geode/pull/6116#discussion_r602616553



##########
File path: geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionIntegrationTest.java
##########
@@ -30,6 +30,12 @@
 @Category(GfshTest.class)
 public class GfshParserAutoCompletionIntegrationTest {
 
+  /**
+   * Number of @CliOption parameters of StartServerCommand.startServer()
+   * method +1 due to "--group" & "--groups" are defined in the same @CliOption
+   */
+  final int startServerCommandCliOptions = 55;

Review comment:
       Can this be a calculated rather than magic number?

##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
##########
@@ -953,8 +955,22 @@ public void init(Services<ID> s) throws MembershipConfigurationException {
   public void started() throws MemberStartupException {
     setLocalAddress(services.getMessenger().getMemberID());
     try {
-      serverSocket = createServerSocket(localAddress.getInetAddress(),
-          services.getConfig().getMembershipPortRange());
+      InetAddress address = localAddress.getInetAddress();

Review comment:
       I think it would be good to make this a separate function. This clutters up a function that doesn't really care.




-- 
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:
users@infra.apache.org