curcur commented on pull request #11725: URL: https://github.com/apache/flink/pull/11725#issuecomment-629650067
Hey @AHeise , thank you so much again! I believe I've addressed most of the minor issues mentioned in the previous reviews. Please remind me if I miss anything but for the one "extracted this inside of the for loop". I've left an offline msg to you to explain why I think it is better not to do it right now. FLINK-17307 changes the KafkaFetcher to emit the output in batch (once collector per partition), but mine is using once per record (emitting record and watermark in a different way). You can see they have logical differences. It might worth changing to batch emit as well, but might be two collectors, one for watermarks and the other one for records. I am not saying the change is difficult, but I am learning to be conservative right now and I've already listed it as a follow-up: https://issues.apache.org/jira/browse/FLINK-15670. What do you think? I am going to work on the Java docs for the public API now. If you have any comments on the code, please let me know. ---------------------------------------------------------------- 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
