DanielLeens commented on PR #11586: URL: https://github.com/apache/seatunnel/pull/11586#issuecomment-5363666548
Thanks @SEZ9 for consolidating this into a concrete checklist — that's genuinely helpful and I think it gives @TFGSUMIT the clearest picture yet of what's needed. From Daniel's side: I agree with all four items above, and nothing new needs adding to the list. The head is still `cb3017b6` (unchanged since my last note), so there's nothing further to re-review right now — I'll hold off on a fresh full review until an updated commit lands. @TFGSUMIT, to be very concrete about sequencing: item 1 and 2 (real `seatunnel-api` module, valid Java syntax) have to come first, since the code won't compile otherwise — CI (item 4) can't tell you anything useful until then. Item 3 (the `Builder.required(...)` reassignment bug) is worth fixing in the same pass since it's a one-line-scope change once the code is in the right place. Once you push, tag either of us and we'll do a complete re-review of the new diff. No rush — take the time you need, and ask here if any of the four points aren't clear. -- 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]
