pandaapo commented on code in PR #4698:
URL: https://github.com/apache/eventmesh/pull/4698#discussion_r1438492842


##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/file/WatchFileManager.java:
##########
@@ -62,10 +60,10 @@ private static void shutdown() {
             return;
         }
 
-        LogUtils.info(log, "[WatchFileManager] start close");
+        log.atInfo().log("[WatchFileManager] start close");

Review Comment:
   1. Since `log.info()` already supports determining log level internally, why 
not use a more concise `log.info()` here?
   
   2. Why not use the solution provided by @ppkarwasz in the 
[discussion](https://github.com/apache/logging-log4j2/discussions/2133) to 
modify LogUtils? Isn't this change smaller?
   ```
       private static final String FQCN = LogUtils.class.getName():
   
       public static void trace(Logger logger, Marker marker, String msg, 
Object arg) {
           final LoggingEventBuilder builder = logger.atTrace();
           if (builder instanceof CallerBoundaryAware) {
               ((CallerBoundaryAware) builder).setCallerBoundary(FQCN);
           }
          builder.addMarker(marker).log(msg, arg);
       }
   ```
   
   ---
   1. 既然`log.info()`内部已经支持判断日志级别,为什么这里不直接使用更简洁的`log.info()`?
   2. 为什么不采用[讨论](https://github.com/apache/logging-log4j2/discussions/2133)中 
ppkarwasz 提供的这种方案修改下LogUtils,这样改动不是更小吗?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to