----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27693/#review61873 -----------------------------------------------------------
Good work! Left some comments. core/src/main/scala/kafka/tools/ConsumerCommand.scala <https://reviews.apache.org/r/27693/#comment103797> NIT: Not sure if there is any standard we have around max line length, but having small line size usually makes code mroe readable. core/src/main/scala/kafka/tools/ConsumerCommand.scala <https://reviews.apache.org/r/27693/#comment103798> Any particular reason we are hiding stacktrace here? Adding message is always good, but stacktrace is always more useful :) core/src/main/scala/kafka/tools/ConsumerCommand.scala <https://reviews.apache.org/r/27693/#comment103796> Given that ConsumerCommand serves multiple functionality, .i.e., prints multiple information, it is important that we provide information on what we are printing. Something like, "Available consumer groups", is required here. core/src/main/scala/kafka/tools/ConsumerCommand.scala <https://reviews.apache.org/r/27693/#comment103801> Same as above about not hiding ST. core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/27693/#comment103790> Use ZkUtils.ConsumersPath - Ashish Singh On Nov. 10, 2014, 7:06 p.m., Balaji Seshadri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27693/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 7:06 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1476 > https://issues.apache.org/jira/browse/KAFKA-1476 > > > Repository: kafka > > > Description > ------- > > KAFKA-1476 Get list of consumer groups - Review Comments > > > Diffs > ----- > > core/src/main/scala/kafka/tools/ConsumerCommand.scala PRE-CREATION > core/src/main/scala/kafka/utils/ZkUtils.scala > 56e3e88e0cc6d917b0ffd1254e173295c1c4aabd > > Diff: https://reviews.apache.org/r/27693/diff/ > > > Testing > ------- > > > Thanks, > > Balaji Seshadri > >
