Re: Review Request 27634: Patch for KAFKA-1667

2014-11-25 Thread Dmytro Kostiuchenko
> On Nov. 21, 2014, 7:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogConfig.scala, lines 176-177 > > > > > > This is a useful genenal check for boolean type. Could we include the > > validation in Config

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-25 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 25, 2014, 11:04 a.m.) Review request for kafka. Bugs: KAFKA-166

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-21 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review62608 --- Thanks for the new patch. A few more comments below. core/src/main

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-16 Thread Dmytro Kostiuchenko
> On Nov. 15, 2014, 1:48 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogConfig.scala, line 46 > > > > > > This is actually a hard limit. All docs are just copied from javadoc. I've changed nothing, and thus

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-16 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 16, 2014, 5:33 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-16 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 16, 2014, 5:31 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-14 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review61581 --- Thanks for the patch. Looks good overall. A few comments below. co

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-12 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 12, 2014, 11:49 a.m.) Review request for kafka. Bugs: KAFKA-166

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-11 Thread Dmytro Kostiuchenko
> On Nov. 7, 2014, 5 a.m., Gwen Shapira wrote: > > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala, > > line 144 > > > > > > String.valueOf(false) should evaluate to "false", right? Why do w

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-10 Thread Gwen Shapira
> On Nov. 7, 2014, 5 a.m., Gwen Shapira wrote: > > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala, > > line 144 > > > > > > String.valueOf(false) should evaluate to "false", right? Why do w

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-10 Thread Gwen Shapira
> On Nov. 7, 2014, 5 a.m., Gwen Shapira wrote: > > core/src/main/scala/kafka/utils/Utils.scala, lines 514-522 > > > > > > I think I'm not seeing why we need this. > > Shouldn't Scala's JavaConversion class handle

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-07 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 7, 2014, 1:28 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-07 Thread Dmytro Kostiuchenko
> On Nov. 7, 2014, 5 a.m., Gwen Shapira wrote: > > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala, > > line 144 > > > > > > String.valueOf(false) should evaluate to "false", right? Why do w

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-06 Thread Gwen Shapira
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review60188 --- Awesome. Its building successfully now. core/src/main/scala/kafka/

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-06 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 6, 2014, 4:12 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-06 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 6, 2014, 4:10 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-05 Thread Gwen Shapira
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review59997 --- I like how you added validations while preserving the API to minimiz

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-05 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 5, 2014, 6:44 p.m.) Review request for kafka. Bugs: KAFKA-1667

Re: Review Request 27634: Patch for KAFKA-1667

2014-11-05 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- (Updated Nov. 5, 2014, 6:42 p.m.) Review request for kafka. Bugs: KAFKA-1667

Review Request 27634: Patch for KAFKA-1667

2014-11-05 Thread Dmytro Kostiuchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/ --- Review request for kafka. Bugs: KAFKA-1667 https://issues.apache.org/jira/b