You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Mayuresh Gharat <gh...@gmail.com> on 2014/09/02 22:42:12 UTC

Re: Review Request 25136: Patch for KAFKA-1610


> On Aug. 29, 2014, 9:01 p.m., Guozhang Wang wrote:
> > LGTM. One minor thing about comments is that we do not need to say "Changing mapValues to map" since we do not need to leave comments indicating code change, but just comment on the purpose of coding. We can generally say sth. like "Create a new collection with map since it (maybe modified outside this block / in another function call, etc)".

Cool. I will change those comments and upload a new patch. Thanks.


- Mayuresh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25136/#review51909
-----------------------------------------------------------


On Aug. 29, 2014, 5:04 p.m., Mayuresh Gharat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25136/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2014, 5:04 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1610
>     https://issues.apache.org/jira/browse/KAFKA-1610
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Added comments explaining the changes and reverted back some changes as per comments on the reviewboard
> 
> 
> Removed the unnecessary import
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 691d69a49a240f38883d2025afaec26fd61281b5 
>   core/src/main/scala/kafka/controller/KafkaController.scala 8ab4a1b8072c9dd187a9a6e94138b725d1f1b153 
>   core/src/main/scala/kafka/server/DelayedFetch.scala e0f14e25af03e6d4344386dcabc1457ee784d345 
>   core/src/main/scala/kafka/server/DelayedProduce.scala 9481508fc2d6140b36829840c337e557f3d090da 
>   core/src/main/scala/kafka/server/KafkaApis.scala c584b559416b3ee4bcbec5966be4891e0a03eefb 
>   core/src/main/scala/kafka/server/KafkaServer.scala 28711182aaa70eaa623de858bc063cb2613b2a4d 
>   core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala af4783646803e58714770c21f8c3352370f26854 
>   core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala c2ba07c5fdbaf0e65ca033b2e4d88f45a8a15b2e 
> 
> Diff: https://reviews.apache.org/r/25136/diff/
> 
> 
> Testing
> -------
> 
> Ran the unit tests and everything passed and the build succeeeded
> 
> 
> Thanks,
> 
> Mayuresh Gharat
> 
>