Bill commented on a change in pull request #6298:
URL: https://github.com/apache/geode/pull/6298#discussion_r612044863



##########
File path: 
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGAddress.java
##########
@@ -91,9 +91,9 @@ public String toString() {
     StringBuilder sb = new StringBuilder();
 
     if (ip_addr == null)
-      sb.append("<null>");
+      sb.append("<no address>");
     else {
-      sb.append(ip_addr.getHostName());
+      sb.append(ip_addr);

Review comment:
       yes

##########
File path: 
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
##########
@@ -335,7 +335,9 @@ public void start() throws MemberStartupException {
     try {
       Object oldDSMembershipInfo = services.getConfig().getOldMembershipInfo();
       if (oldDSMembershipInfo != null) {
-        logger.debug("Reusing JGroups channel from previous system", 
properties);
+        if (logger.isDebugEnabled()) {

Review comment:
       This extra conditional (and the others like it) don't seem to improve 
performance or maintainability. Is there a coding standard for Geode, that 
necessitates this pre-checking?
   
   I searched the codebase for e.g. "logger.debug(" calls and while many do the 
pre-check like this, many others do not.




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


Reply via email to