Return-Path: X-Original-To: apmail-kafka-dev-archive@www.apache.org Delivered-To: apmail-kafka-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 49FDE1021F for ; Tue, 18 Nov 2014 05:24:32 +0000 (UTC) Received: (qmail 36709 invoked by uid 500); 18 Nov 2014 05:24:32 -0000 Delivered-To: apmail-kafka-dev-archive@kafka.apache.org Received: (qmail 36658 invoked by uid 500); 18 Nov 2014 05:24:31 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 36641 invoked by uid 99); 18 Nov 2014 05:24:31 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 18 Nov 2014 05:24:31 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 0D6701DF2D0; Tue, 18 Nov 2014 05:24:31 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============4578281811991289095==" MIME-Version: 1.0 Subject: Re: Review Request 27693: Patch for KAFKA-1476 From: "Ashish Singh" To: "kafka" , "Ashish Singh" , "Balaji Seshadri" Date: Tue, 18 Nov 2014 05:24:31 -0000 Message-ID: <20141118052431.24716.23770@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Ashish Singh" X-ReviewGroup: kafka X-ReviewRequest-URL: https://reviews.apache.org/r/27693/ X-Sender: "Ashish Singh" References: <20141110190644.19819.65143@reviews.apache.org> In-Reply-To: <20141110190644.19819.65143@reviews.apache.org> Reply-To: "Ashish Singh" X-ReviewRequest-Repository: kafka --===============4578281811991289095== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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 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 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 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 Same as above about not hiding ST. core/src/main/scala/kafka/utils/ZkUtils.scala 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 > > --===============4578281811991289095==--