----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32841/#review92753 -----------------------------------------------------------
log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java (line 115) <https://reviews.apache.org/r/32841/#comment146997> Would it not be better to take a `Class<? extends Keyer>` here? log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java (line 157) <https://reviews.apache.org/r/32841/#comment146996> Code convention: space before and after `?` and `:`. log4j-appender/src/main/java/org/apache/kafka/log4jappender/Keyer.java (line 26) <https://reviews.apache.org/r/32841/#comment146999> `Keyer` is a bit of an unusual name, but it's very concise. Another alternative could be `KeyProvider`. Let's see what others say though. - Ismael Juma On July 18, 2015, 2:46 a.m., benoyantony wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32841/ > ----------------------------------------------------------- > > (Updated July 18, 2015, 2:46 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2041 > https://issues.apache.org/jira/browse/KAFKA-2041 > > > Repository: kafka > > > Description > ------- > > KAFKA-2041 > > > Diffs > ----- > > > log4j-appender/src/main/java/org/apache/kafka/log4jappender/KafkaLog4jAppender.java > 628ff53 > log4j-appender/src/main/java/org/apache/kafka/log4jappender/Keyer.java > PRE-CREATION > > log4j-appender/src/test/java/org/apache/kafka/log4jappender/KafkaLog4jAppenderTest.java > 71bdd94 > log4j-appender/src/test/java/org/apache/kafka/log4jappender/MockKeyer.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/32841/diff/ > > > Testing > ------- > > > Thanks, > > benoyantony > >
