ppkarwasz commented on code in PR #1985:
URL: https://github.com/apache/logging-log4j2/pull/1985#discussion_r1402076017
##########
log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/categories/Appenders.java:
##########
@@ -24,13 +24,5 @@ interface AsyncConversant {}
interface AsyncJcTools {}
- interface Cassandra {}
-
- interface CouchDb {}
-
- interface Kafka {}
-
- interface MongoDb {}
-
interface ZeroMq {}
Review Comment:
Probably we also need to remove `ZeroMq`.
##########
log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayout.java:
##########
@@ -249,6 +250,11 @@ public Map<String, String> getContentFormat() {
return CONTENT_FORMAT;
}
+ public static JsonTemplateLayout createDefaultLayout() {
+ final DefaultConfiguration configuration = new DefaultConfiguration();
+ return
JsonTemplateLayout.newBuilder().setConfiguration(configuration).build();
+ }
Review Comment:
I am not totally comfortable with `new DefaultConfiguration()` which creates
a console appender, a pattern layout and other components.
I would rather provide a `Configuration` as parameter to the method. If the
parameter is `null`, the layout will have no configuration to use for
interpolation.
##########
log4j-core-its/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java:
##########
@@ -37,17 +41,13 @@
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LoggingException;
import org.apache.logging.log4j.ThreadContext;
-import org.apache.logging.log4j.core.Appender;
-import org.apache.logging.log4j.core.LogEvent;
-import org.apache.logging.log4j.core.Logger;
-import org.apache.logging.log4j.core.LoggerContext;
-import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.*;
Review Comment:
IMHO wildcard imports should not be used.
Since the Palantir formatter is neutral on this regard, I am afraid we all
need to disable them in our IDEs.
We can also add a Maven Enforcer rule to keep from reverting to wildcard
imports (e.g. https://github.com/skuzzle/restrict-imports-enforcer-rule ).
##########
log4j-distribution/pom.xml:
##########
@@ -63,11 +63,6 @@
<artifactId>snappy-java</artifactId>
<version>${snappy.version}</version>
</dependency>
- <dependency>
Review Comment:
I would remove the whole `log4j-distribution`.
--
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]
