matriv commented on pull request #18479: URL: https://github.com/apache/flink/pull/18479#issuecomment-1023251677
Having the `id` as before, doesn't add `complication`, just adds another argument to all constructors, so I can still revert the and remove it from `ExecNodeContext`, just needs some more work, to fix all the nodes in the hierarchy. On one hand, I think that accessing the `id` through the `Context` is nice, and we have everything in one place + the operator uid in the future. On the other hand, I agree that this way, we are mixing the id together with the mandatory info for the `type`. I also don't really have a strong opinion, just a slight preference to have everything in the POJO, but please let me know. If you both prefer a separate `id`, I'm happy to do it. @twalthr @slinkydeveloper -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
