You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/06/04 10:16:19 UTC

[GitHub] [logging-log4j2] vy opened a new pull request #504: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible

vy opened a new pull request #504:
URL: https://github.com/apache/logging-log4j2/pull/504


   


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



[GitHub] [logging-log4j2] vy merged pull request #504: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible

Posted by GitBox <gi...@apache.org>.
vy merged pull request #504:
URL: https://github.com/apache/logging-log4j2/pull/504


   


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



[GitHub] [logging-log4j2] vy commented on a change in pull request #504: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible

Posted by GitBox <gi...@apache.org>.
vy commented on a change in pull request #504:
URL: https://github.com/apache/logging-log4j2/pull/504#discussion_r646334274



##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
##########
@@ -507,11 +508,27 @@ public void log(final String fqcn, final Priority priority, final Object message
         }
     }
 
-    private void maybeLog(final String fqcn, final org.apache.logging.log4j.Level level,
-            final Object message, final Throwable throwable) {
+    private void maybeLog(
+            final String fqcn,
+            final org.apache.logging.log4j.Level level,
+            final Object message,
+            final Throwable throwable) {
         if (logger.isEnabled(level)) {
-            @SuppressWarnings("unchecked")
-            Message msg = message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message);
+            final Message msg;
+            if (message instanceof String) {
+                msg = new SimpleMessage((String) message);
+            }
+            // SimpleMessage treats String and CharSequence differently, hence
+            // this else-if block.
+            else if (message instanceof CharSequence) {
+                msg = new SimpleMessage((CharSequence) message);
+            } else if (message instanceof Map) {
+                @SuppressWarnings("unchecked")
+                final Map<String, ?> map = (Map<String, ?>) message;

Review comment:
       It is there to suppress `javac` warnings via `@SuppressWarnings("unchecked")`.




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



[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #504: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #504:
URL: https://github.com/apache/logging-log4j2/pull/504#discussion_r645655510



##########
File path: log4j-1.2-api/src/main/java/org/apache/log4j/Category.java
##########
@@ -507,11 +508,27 @@ public void log(final String fqcn, final Priority priority, final Object message
         }
     }
 
-    private void maybeLog(final String fqcn, final org.apache.logging.log4j.Level level,
-            final Object message, final Throwable throwable) {
+    private void maybeLog(
+            final String fqcn,
+            final org.apache.logging.log4j.Level level,
+            final Object message,
+            final Throwable throwable) {
         if (logger.isEnabled(level)) {
-            @SuppressWarnings("unchecked")
-            Message msg = message instanceof Map ? new MapMessage((Map) message) : new ObjectMessage(message);
+            final Message msg;
+            if (message instanceof String) {
+                msg = new SimpleMessage((String) message);
+            }
+            // SimpleMessage treats String and CharSequence differently, hence
+            // this else-if block.
+            else if (message instanceof CharSequence) {
+                msg = new SimpleMessage((CharSequence) message);
+            } else if (message instanceof Map) {
+                @SuppressWarnings("unchecked")
+                final Map<String, ?> map = (Map<String, ?>) message;

Review comment:
       Is the map local var really needed?




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



[GitHub] [logging-log4j2] vy commented on pull request #504: LOG4J2-3080 Use SimpleMessage in Log4j 1 Category whenever possible

Posted by GitBox <gi...@apache.org>.
vy commented on pull request #504:
URL: https://github.com/apache/logging-log4j2/pull/504#issuecomment-855669846


   @garydgregory @carterkozak @rgoers Thanks for the review. Will merge this and port to `master`.


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