> On March 24, 2015, 5 p.m., Mayuresh Gharat wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 134 > > <https://reviews.apache.org/r/32440/diff/1/?file=904417#file904417line134> > > > > Since its a Producer level config, is this change needed. We can keep > > it as an instance variable. Also since the compression type does not > > change, the "private final" makes it more clear. What do you think? > > Grant Henke wrote: > I don't mind leaving the instance level config. However, since it is not > used anywhere but the constructor I don't see the value in it. If we want to > mark it as final we can in the constructor and have the same clarity. The > only reason I didn't initially is because the other code did not seam to > follow the style of putting final on everything. (Note: I would prefer to put > final on everything)
Completely agree that its not used as much. Not having it as instance level config is not going to effect the functionality, just that someone would prefer the configs to be together. - Mayuresh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32440/#review77593 ----------------------------------------------------------- On March 24, 2015, 3:51 p.m., Grant Henke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32440/ > ----------------------------------------------------------- > > (Updated March 24, 2015, 3:51 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2043 > https://issues.apache.org/jira/browse/KAFKA-2043 > > > Repository: kafka > > > Description > ------- > > CompressionType is passed in each RecordAccumulator append > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > feda9c922d7dab17e424f8e6f0aa0a3f968e3729 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > 88b4e4fbf3bf6fb6d2f90551a792b95d4cd51c40 > > clients/src/test/java/org/apache/kafka/clients/producer/internals/RecordAccumulatorTest.java > e379ac89c9a2fbfe750d6b0dec693b7eabb76204 > > clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java > 24274a64885fadd0e9318de2beb232218ddd52cd > > Diff: https://reviews.apache.org/r/32440/diff/ > > > Testing > ------- > > > Thanks, > > Grant Henke > >
