----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28536/#review63295 -----------------------------------------------------------
Thanks for the patch. Some comments below. Also, could we add a unit test for getConfiguredInstances()? clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java <https://reviews.apache.org/r/28536/#comment105451> Do we really need to special case this? If we don't handle this, we will get a ConfigException anyway. clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java <https://reviews.apache.org/r/28536/#comment105450> We probably should throw a ConfigException instead. - Jun Rao On Nov. 29, 2014, 10:20 a.m., Manikumar Reddy O wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28536/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2014, 10:20 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1799 > https://issues.apache.org/jira/browse/KAFKA-1799 > > > Repository: kafka > > > Description > ------- > > Explicit string to class conversion done in > gAbstractConfig.getConfiguredInstances() > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > 8d886105341555a548ecc7b2901e7fc5d6b1ee8c > > Diff: https://reviews.apache.org/r/28536/diff/ > > > Testing > ------- > > > Thanks, > > Manikumar Reddy O > >
