----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23567/#review50081 -----------------------------------------------------------
core/src/main/scala/kafka/api/TransactionRequest.scala <https://reviews.apache.org/r/23567/#comment87615> Although this is "sort-of" a constructor it is a regular method so start with lower case. Also, maybe call it transactionRequestWithNewControl core/src/main/scala/kafka/api/TransactionRequest.scala <https://reviews.apache.org/r/23567/#comment87613> commit vs pre-commit: I think this is needed for the txcoordinator to broker instruction. i.e., there is a distinction between what is stored in the log and the inter-broker communcation. I'm wondering if we should separate these better. OTOH if we were to separate these they should ideally be consistent (i.e., same value) which is easier if they are all in one place - what do you think? core/src/main/scala/kafka/api/TransactionRequest.scala <https://reviews.apache.org/r/23567/#comment87616> From the comments in the earlier version of the patch we concluded that you did not need the order - can we just do a simple groupBy? core/src/main/scala/kafka/api/TransactionRequest.scala <https://reviews.apache.org/r/23567/#comment87617> Same as above - thought this was no longer required (see comments from previous review) - Joel Koshy On Aug. 6, 2014, 4:28 a.m., Dong Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23567/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2014, 4:28 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1522 > https://issues.apache.org/jira/browse/KAFKA-1522 > > > Repository: kafka > > > Description > ------- > > KAFKA-1522 Tansactional messaging request/response definitions > > > Diffs > ----- > > core/src/main/scala/kafka/api/RequestKeys.scala > c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 > core/src/main/scala/kafka/api/TransactionRequest.scala PRE-CREATION > core/src/main/scala/kafka/api/TransactionResponse.scala PRE-CREATION > core/src/main/scala/kafka/api/TxCoordinatorMetadataRequest.scala > PRE-CREATION > core/src/main/scala/kafka/api/TxCoordinatorMetadataResponse.scala > PRE-CREATION > core/src/main/scala/kafka/common/ErrorMapping.scala > 5559d26ba2b96059f719754a351fa4598ca8a70b > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > cd16ced5465d098be7a60498326b2a98c248f343 > > Diff: https://reviews.apache.org/r/23567/diff/ > > > Testing > ------- > > > Thanks, > > Dong Lin > >
