curcur edited a comment 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 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. As it is only related to putting `for loops` inside or outside of the extraction, it is not that dirty? What do you think? I am going to work on the Java Docs for public API right now. Please let me know if you have any more comments on the code side. Thanks!!:grin: ---------------------------------------------------------------- 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: [email protected]
