Re: Review Request 28025: Patch for KAFKA-345

2014-11-18 Thread Joel Koshy
> On Nov. 19, 2014, 12:01 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala, lines > > 166-169 > > > > > > Should this be added to the ConsumerConnector interface? > > >

Re: Review Request 28025: Patch for KAFKA-345

2014-11-18 Thread Jiangjie Qin
> On Nov. 18, 2014, 6:28 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, > > line 47 > > > > > > * I think we can just combine this patch with the mirror maker >

Re: Review Request 28025: Patch for KAFKA-345

2014-11-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/#review62058 --- Thanks for the patch. A few comments below. core/src/main/scala/ka

Re: Review Request 28025: Patch for KAFKA-345

2014-11-18 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/#review61964 --- Overall this looks good except for a naming suggestion and removal o

Re: Review Request 28025: Patch for KAFKA-345

2014-11-17 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/ --- (Updated Nov. 18, 2014, 1:42 a.m.) Review request for kafka. Bugs: KAFKA-345

Re: Review Request 28025: Patch for KAFKA-345

2014-11-17 Thread Joel Koshy
> On Nov. 14, 2014, 11:36 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, > > line 29 > > > > > > It would be worth elaborating the use-case for these hooks (i.e.,

Re: Review Request 28025: Patch for KAFKA-345

2014-11-15 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/ --- (Updated Nov. 15, 2014, 9:20 a.m.) Review request for kafka. Bugs: KAFKA-345

Re: Review Request 28025: Patch for KAFKA-345

2014-11-15 Thread Jiangjie Qin
> On Nov. 14, 2014, 11:36 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/javaapi/consumer/ConsumerRebalanceListener.java, > > line 24 > > > > > > Can we instead just have an interface for this? We don't need to

Re: Review Request 28025: Patch for KAFKA-345

2014-11-15 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/ --- (Updated Nov. 15, 2014, 9:01 a.m.) Review request for kafka. Bugs: KAFKA-345

Re: Review Request 28025: Patch for KAFKA-345

2014-11-14 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/#review61556 --- core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala

Review Request 28025: Patch for KAFKA-345

2014-11-13 Thread Jiangjie Qin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28025/ --- Review request for kafka. Bugs: KAFKA-345 https://issues.apache.org/jira/br