----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28096/#review94522 -----------------------------------------------------------
core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 107) <https://reviews.apache.org/r/28096/#comment149146> nit: ``numInterations`` don't need to be a ``var`` (the less vars the better, right?) It's a matter of deleting L#107, making L#109 as ``val numIterations = opts.options.valueOf(opts.numIterationsOpt).toInt`` and moving L#117 to L#113 (shift former L#113 down). This has the benefit of making the ``return`` keyword unnecessary at L#114. That is: `` if (opts.options.has(opts.numIterationsOpt)) { val numIterations = opts.options.valueOf(opts.numIterationsOpt).toInt if (numIterations < 0) CommandLineUtils.printUsageAndDie(opts.parser, s"Loop value must be greater than 0:${opts.options.valueOf(opts.loopIntervalOpt)}") currentIteration < numIterations } else { true // keep iterating, if iterations not limited by numIterations } `` core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 335) <https://reviews.apache.org/r/28096/#comment149149> single line commands don't use brackets, right? L#335 and L#338. core/src/main/scala/kafka/admin/ConsumerGroupCommand.scala (line 345) <https://reviews.apache.org/r/28096/#comment149145> nit: you could rename this as ``elapsedTime`` and make it ``System.currentTimeMillis() - startTime`` as it's only used at line L#346. So, L#346 becomes ``val sleepTime = loopInterval - elapsedTime``. - Edward Ribeiro On Aug. 5, 2015, 10:43 p.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28096/ > ----------------------------------------------------------- > > (Updated Aug. 5, 2015, 10:43 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 > >
