> On June 11, 2015, 10:15 p.m., Aditya Auradkar wrote: > > Hey Ashish, > > > > I've left a few minor comments. Thanks!
Thanks for the review Aditya. Addressed your comments. > On June 11, 2015, 10:15 p.m., Aditya Auradkar wrote: > > log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java, > > line 135 > > <https://reviews.apache.org/r/33614/diff/6/?file=946526#file946526line135> > > > > perhaps wrap this inside an isDebugEnabled check? That check is anyways done my the method itself, https://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Category.html#debug(java.lang.Object). > On June 11, 2015, 10:15 p.m., Aditya Auradkar wrote: > > log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java, > > line 124 > > <https://reviews.apache.org/r/33614/diff/6/?file=946526#file946526line124> > > > > can this be logged as info since it's infrequent? Based on Jun's comment, I am now using LogLog, which does not have an info level logging support. I also do not think it will be a blocker if this remains a debug level message. Let me know if you think otherwise. - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33614/#review87638 ----------------------------------------------------------- On June 14, 2015, 4:19 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33614/ > ----------------------------------------------------------- > > (Updated June 14, 2015, 4:19 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2132 > https://issues.apache.org/jira/browse/KAFKA-2132 > > > Repository: kafka > > > Description > ------- > > KAFKA-2132: Move Log4J appender to clients module > > > Diffs > ----- > > build.gradle 30d1cf2f1ff9ed3f86a060da8099bb0774b4cf91 > checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 > core/src/main/scala/kafka/producer/KafkaLog4jAppender.scala > 5d36a019e3dbfb93737a9cd23404dcd1c5d836d1 > core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala > 41366a14590d318fced0e83d6921d8035fa882da > > log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java > PRE-CREATION > > log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java > PRE-CREATION > > log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKafkaLog4jAppender.java > PRE-CREATION > settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 > > Diff: https://reviews.apache.org/r/33614/diff/ > > > Testing > ------- > > > Thanks, > > Ashish Singh > >
