> On Jan. 8, 2014, 2:49 a.m., joel koshy wrote: > > core/src/main/scala/kafka/utils/Utils.scala, line 544 > > <https://reviews.apache.org/r/16718/diff/1/?file=418272#file418272line544> > > > > Thanks for patching this issue. > > > > I'm not very clear on the cases here - i.e., is it exhaustive? Also, > > why is this method named quote"Json"Literal? > > > > Finally, it may be a good idea to add more test cases in > > TopicFilterTest. > > Joe Stein wrote: > Thanks Joel for reviewing this. It is exhaustive http://json.org/ and > the name of the function needs to get changed, yup! Will also get a test > done.
It in addition to the suggestions that Joel raised, it will be very helpful if you can add documentation for the quoteJsonLiteral so people can understand why we added it. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16718/#review31336 ----------------------------------------------------------- On Jan. 8, 2014, 1:29 a.m., Joe Stein wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16718/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2014, 1:29 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1180 > https://issues.apache.org/jira/browse/KAFKA-1180 > > > Repository: kafka > > > Description > ------- > > KAFKA-1180 WhiteList topic filter gets a NullPointerException on complex Regex > > > Diffs > ----- > > core/src/main/scala/kafka/consumer/TopicCount.scala > e33263378489f7cb5ba98476a8e6d65640130965 > core/src/main/scala/kafka/utils/Utils.scala > a89b0463685e6224d263bc9177075e1bb6b93d04 > > Diff: https://reviews.apache.org/r/16718/diff/ > > > Testing > ------- > > > Thanks, > > Joe Stein > >
