----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28859/#review64546 -----------------------------------------------------------
Non-binding LGTM from me. Any chance of cleaning up the whitespace? makes the change looks way bigger than it really is. Few suggested improvements below: core/src/main/scala/kafka/utils/Utils.scala <https://reviews.apache.org/r/28859/#comment107303> Scala has Pair type. Since we always expect 2 elements, it will be more descriptive and safer to use it instead of the internal Array. core/src/test/scala/unit/kafka/utils/UtilsTest.scala <https://reviews.apache.org/r/28859/#comment107304> Since this is a generic function, I'd add tests for: 1. list with a single host:port pair (i.e. no commas) 2. ipv4 - Gwen Shapira On Dec. 9, 2014, 6:11 p.m., Jeff Holoman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28859/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2014, 6:11 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1812 > https://issues.apache.org/jira/browse/KAFKA-1812 > > > Repository: kafka > > > Description > ------- > > KAFKA-1812 initial > > > Diffs > ----- > > core/src/main/scala/kafka/utils/Utils.scala > 58685cc47b4c43e4ee68b73f1ee34eb99a5aa547 > core/src/test/scala/unit/kafka/utils/UtilsTest.scala > 0d0f0e2fba367180eeb718a259e8d680a73c3a73 > > Diff: https://reviews.apache.org/r/28859/diff/ > > > Testing > ------- > > > Thanks, > > Jeff Holoman > >
