SEZ9 commented on PR #12079:
URL: https://github.com/apache/seatunnel/pull/12079#issuecomment-5806545270

   Thanks for pulling those points out — the mapping of F1, F2/F4, F3/F5 and F6 
to the changes in `d8045271fcf6` is helpful. I'll verify each against the diff 
on that head before marking them resolved rather than closing them from the 
summary alone.
   
   Two items from the original review don't appear to be covered yet — could 
you confirm where they were handled, or whether they're still open?
   
   1. **F7 (docs):** Rejecting cyclic/unresolved transform graphs with a 
`ConfigCheckException` is a user-facing change for configs that were previously 
accepted. A short docs/changelog/upgrade note describing the new error and how 
to fix the config (adding explicit `plugin_input`/`plugin_output`) would help.
   2. **F8 (style):** Is the `getOutputId()` accessor used in production code 
or only by tests, and were the static input/output-ID helpers in the dry-run 
validator removed in favour of the shared ones so `ReadonlyConfig` is no longer 
re-parsed?
   
   Once F7 and F8 are settled I'm happy to approve.
   
   <!-- streview-comment:1273 -->


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to