----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35086/#review88131 -----------------------------------------------------------
This looks good to me. Just have a minor comment below, and one orthogonal comment. FWIW, I did not get any conflicts while applying this patch. core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala (line 49) <https://reviews.apache.org/r/35086/#comment140543> Orthogonal to this patch, but I really think we should protect this requirement by code rather than comment. core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala (line 63) <https://reviews.apache.org/r/35086/#comment140929> rather than redeclare these everywhere, can we just move this up to a member val and reuse in the various tests unless explicitly overridden? - Joel Koshy On June 4, 2015, 7:57 p.m., Onur Karaman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35086/ > ----------------------------------------------------------- > > (Updated June 4, 2015, 7:57 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2245 > https://issues.apache.org/jira/browse/KAFKA-2245 > > > Repository: kafka > > > Description > ------- > > add response tests for ConsumerCoordinator > > > Diffs > ----- > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala > 51e89c87ee2c20fc7f976536f01fa1055fb8e670 > core/src/main/scala/kafka/coordinator/CoordinatorMetadata.scala > c39e6de34ee531c6dfa9107b830752bd7f8fbe59 > > core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/35086/diff/ > > > Testing > ------- > > > Thanks, > > Onur Karaman > >
