----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28096/#review93489 -----------------------------------------------------------
Thanks for the patch and sorry for the long delay. core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (lines 36 - 39) <https://reviews.apache.org/r/28096/#comment147867> Shouldn't this be an inner object? since its only visible and used by ConsumerGroupCommand? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 93) <https://reviews.apache.org/r/28096/#comment147875> This is defined as "breakable", but I don't see where you use break? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (lines 109 - 113) <https://reviews.apache.org/r/28096/#comment147878> Since its a boolean, we want a name that reflects what we check. Perhaps "iterationsLeft"? it looks like we want to return true if we want to continue iterating - in this case shouldn't a negative numIterations lead to false? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (lines 236 - 241) <https://reviews.apache.org/r/28096/#comment147880> These look identical - copy/paste error? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 249) <https://reviews.apache.org/r/28096/#comment147881> I thought none is tab-delimited, not CSV? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 363) <https://reviews.apache.org/r/28096/#comment147871> Kafka code base usually doesn't use methods as operators. Why are we doing this here? Also, why define "ofTypes" here? core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 388) <https://reviews.apache.org/r/28096/#comment147883> If only CSV and JSON are allowed, what is NONE for? - Gwen Shapira On June 24, 2015, 6:14 p.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28096/ > ----------------------------------------------------------- > > (Updated June 24, 2015, 6:14 p.m.) > > > Review request for kafka, Gwen Shapira, Jarek Cecho, and Joel Koshy. > > > Bugs: KAFKA-313 > https://issues.apache.org/jira/browse/KAFKA-313 > > > Repository: kafka > > > Description > ------- > > KAFKA-313: Add JSON/CSV output and looping options to ConsumerGroupCommand > > > Diffs > ----- > > core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala > f23120ede5f9bf0cfaf795c65c9845f42d8784d0 > > Diff: https://reviews.apache.org/r/28096/diff/ > > > Testing > ------- > > Ran ConsumerOffsetChecker with different combinations of --output.format and > --loop options. > > > Thanks, > > Ashish Singh > >
