You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2020/07/02 13:24:46 UTC

[GitHub] [httpcomponents-core] carterkozak opened a new pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

carterkozak opened a new pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199


   Methodology:
   I used the intellij idea ultimate edition inspection "non-constant
   string concatenation as argument to logging call" to find and
   auto-fix occurrences of this pattern.
   Followed up with intellij idea fixes for "Unnecessary call to 'toString'"


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199#discussion_r449698055



##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/classic/LoggingHttp1StreamListener.java
##########
@@ -52,9 +52,9 @@ public void onResponseHead(final HttpConnection connection, final HttpResponse r
     public void onExchangeComplete(final HttpConnection connection, final boolean keepAlive) {
         if (connLog.isDebugEnabled()) {
             if (keepAlive) {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is kept alive");
+                connLog.debug("{} Connection is kept alive", LoggingSupport.getId(connection));

Review comment:
       I don't see that pattern in the other listeners, updated to lowercase.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] ok2c merged pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199


   


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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199#discussion_r449128808



##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -76,7 +76,7 @@ public Lock getLock() {
     public void enqueue(final Command command, final Command.Priority priority) {
         this.session.enqueue(command, priority);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Enqueued " + command.getClass().getSimpleName() + " with priority " + priority);
+            this.log.debug("{} Enqueued {} with priority {}", this.session, command.getClass().getSimpleName(), priority);

Review comment:
       Enqueued should probably be lowercase, shouldn't it?

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -133,30 +133,30 @@ private static String formatOps(final int ops) {
     public void setEventMask(final int ops) {
         this.session.setEventMask(ops);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event mask set " + formatOps(ops));
+            this.log.debug("{} Event mask set {}", this.session, formatOps(ops));

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -187,7 +187,7 @@ public Timeout getSocketTimeout() {
     @Override
     public void setSocketTimeout(final Timeout timeout) {
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Set timeout " + timeout);
+            this.log.debug("{} Set timeout {}", this.session, timeout);

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -133,30 +133,30 @@ private static String formatOps(final int ops) {
     public void setEventMask(final int ops) {
         this.session.setEventMask(ops);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event mask set " + formatOps(ops));
+            this.log.debug("{} Event mask set {}", this.session, formatOps(ops));
         }
     }
 
     @Override
     public void setEvent(final int op) {
         this.session.setEvent(op);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event set " + formatOps(op));
+            this.log.debug("{} Event set {}", this.session, formatOps(op));
         }
     }
 
     @Override
     public void clearEvent(final int op) {

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -133,30 +133,30 @@ private static String formatOps(final int ops) {
     public void setEventMask(final int ops) {
         this.session.setEventMask(ops);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event mask set " + formatOps(ops));
+            this.log.debug("{} Event mask set {}", this.session, formatOps(ops));
         }
     }
 
     @Override
     public void setEvent(final int op) {
         this.session.setEvent(op);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event set " + formatOps(op));
+            this.log.debug("{} Event set {}", this.session, formatOps(op));
         }
     }
 
     @Override
     public void clearEvent(final int op) {
         this.session.clearEvent(op);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event cleared " + formatOps(op));
+            this.log.debug("{} Event cleared {}", this.session, formatOps(op));

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -257,7 +257,7 @@ public IOEventHandler getHandler() {
     @Override
     public void upgrade(final IOEventHandler handler) {
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Protocol upgrade: " + (handler != null ? handler.getClass() : null));
+            this.log.debug("{} Protocol upgrade: {}", this.session, handler != null ? handler.getClass() : null);

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -133,30 +133,30 @@ private static String formatOps(final int ops) {
     public void setEventMask(final int ops) {
         this.session.setEventMask(ops);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event mask set " + formatOps(ops));
+            this.log.debug("{} Event mask set {}", this.session, formatOps(ops));
         }
     }
 
     @Override
     public void setEvent(final int op) {
         this.session.setEvent(op);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event set " + formatOps(op));
+            this.log.debug("{} Event set {}", this.session, formatOps(op));
         }
     }
 
     @Override
     public void clearEvent(final int op) {
         this.session.clearEvent(op);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Event cleared " + formatOps(op));
+            this.log.debug("{} Event cleared {}", this.session, formatOps(op));
         }
     }
 
     @Override
     public void close() {
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Close");
+            this.log.debug("{} Close", this.session);

Review comment:
       same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -174,7 +174,7 @@ public boolean isOpen() {
     @Override
     public void close(final CloseMode closeMode) {
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Shutdown " + closeMode);
+            this.log.debug("{} Shutdown {}", this.session, closeMode);

Review comment:
       same here




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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] michael-o commented on a change in pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199#discussion_r449239791



##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingHttp1StreamListener.java
##########
@@ -83,9 +83,9 @@ public void onResponseHead(final HttpConnection connection, final HttpResponse r
     public void onExchangeComplete(final HttpConnection connection, final boolean keepAlive) {
         if (connLog.isDebugEnabled()) {
             if (keepAlive) {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is kept alive");
+                connLog.debug("{} Connection is kept alive", LoggingSupport.getId(connection));

Review comment:
       Same here

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/classic/LoggingHttp1StreamListener.java
##########
@@ -52,9 +52,9 @@ public void onResponseHead(final HttpConnection connection, final HttpResponse r
     public void onExchangeComplete(final HttpConnection connection, final boolean keepAlive) {
         if (connLog.isDebugEnabled()) {
             if (keepAlive) {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is kept alive");
+                connLog.debug("{} Connection is kept alive", LoggingSupport.getId(connection));

Review comment:
       Other messages with id use a colon to separate. If it is such a message use a colon otherwise lowercase connection.

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/classic/LoggingHttp1StreamListener.java
##########
@@ -52,9 +52,9 @@ public void onResponseHead(final HttpConnection connection, final HttpResponse r
     public void onExchangeComplete(final HttpConnection connection, final boolean keepAlive) {
         if (connLog.isDebugEnabled()) {
             if (keepAlive) {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is kept alive");
+                connLog.debug("{} Connection is kept alive", LoggingSupport.getId(connection));
             } else {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is not kept alive");
+                connLog.debug("{} Connection is not kept alive", LoggingSupport.getId(connection));

Review comment:
       Other messages with id use a colon to separate. If it is such a message use a colon otherwise lowercase connection.

##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingHttp1StreamListener.java
##########
@@ -83,9 +83,9 @@ public void onResponseHead(final HttpConnection connection, final HttpResponse r
     public void onExchangeComplete(final HttpConnection connection, final boolean keepAlive) {
         if (connLog.isDebugEnabled()) {
             if (keepAlive) {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is kept alive");
+                connLog.debug("{} Connection is kept alive", LoggingSupport.getId(connection));
             } else {
-                connLog.debug(LoggingSupport.getId(connection) + " Connection is not kept alive");
+                connLog.debug("{} Connection is not kept alive", LoggingSupport.getId(connection));

Review comment:
       Same here




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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] [httpcomponents-core] carterkozak commented on a change in pull request #199: HTTPCORE-636: Logging statements use slf4j interpolation over concatenation

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #199:
URL: https://github.com/apache/httpcomponents-core/pull/199#discussion_r449234336



##########
File path: httpcore5-testing/src/main/java/org/apache/hc/core5/testing/nio/LoggingIOSession.java
##########
@@ -76,7 +76,7 @@ public Lock getLock() {
     public void enqueue(final Command command, final Command.Priority priority) {
         this.session.enqueue(command, priority);
         if (this.log.isDebugEnabled()) {
-            this.log.debug(this.session + " Enqueued " + command.getClass().getSimpleName() + " with priority " + priority);
+            this.log.debug("{} Enqueued {} with priority {}", this.session, command.getClass().getSimpleName(), priority);

Review comment:
       That's true, I've updated these.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org