cmccabe commented on pull request #10069: URL: https://github.com/apache/kafka/pull/10069#issuecomment-775521260
Thanks for this PR, @rondagostino ! I can see why you wanted to have `RaftReplicaChangeDelegateHelper`. The `ReplicaManager` is not very easy to unit test because it has grown so large. I don't think this delegate thing is quite the right abstraction here-- it's pretty confusing-- but I guess let's revisit this after 2.8 is finished. I suppose one option is, once `ReplicaManager` is a pure interface, we can split the kip-500 update logic off into a separate set of functions that takes a `ReplicaManager` as an input. Then we can easily unit-test the update logic with a `MockReplicaManager`. For now I left two small comments... LGTM after those are addressed. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
