[GitHub] [kafka] clolov commented on pull request #13240: KAFKA-14690: Add topic IDs to the OffsetCommit API version 9

["clolov (via GitHub)" <gi...@apache.org>]

clolov commented on PR #13240:
URL: https://github.com/apache/kafka/pull/13240#issuecomment-1553270438

   Heya @dajac! I hope I have addressed your comments on all files except `ConsumerCoordinatorTest` and the `OffsetCommitRequestTest`, could you review everything except those two and confirm whether this is the case? As far as I understand your general concerns with `ConsumerCoordinatorTest` and `OffsetCommitRequestTest` is that those tests are either not parameterised in a simple way or they are not parameterised at all - am I correct? For the ones which are not parameterised simply I cannot think of an easier approach - the setup is just long-winded, but the main idea behind the arguments is https://github.com/apache/kafka/pull/13240/files#diff-964e16515361b7d22acbad4795f13abe6af513be9588952bc6427aa7ce00938dR2875-R2880. I guess we can split them into separate tests were we vary just one of the arguments rather than all of them if that's what you mean? The ones which are not parameterised at all I believe are not parameterised because the same functions are used for tests which onl
 y test behaviour in versions >= 9. I am happy to implement any suggestions you might have to improve on what's already there.


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org