kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ashish Singh" <asi...@cloudera.com>
Subject Re: Review Request 27693: Patch for KAFKA-1476
Date Tue, 18 Nov 2014 05:24:31 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message