SEZ9 commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5658073767
@abdessalems thanks for the rebase and for narrowing this back to the original scope. I can't mark the earlier findings resolved from the thread alone, so I'll hold on that until I've gone through the updated diff: - F1, F2, F4, F5, F6, F8: you note these all targeted the ownership model from the dropped commits (588314d9c, 1fed68145, 912e8bd4c). Once I've confirmed in the diff that none of that code remains here, I'm fine with those moving to the follow-up discussion you mentioned. - F3: I'll verify the Javadoc with the "same monitor as deployment" claim is no longer present. - F7: I'll check that the test no longer uses reflection into internals. On the coverage side, your framing sounds right to me — if the stale-taskDone trigger can't be demonstrated, the test and description shouldn't claim to cover it. Describing the latch release and the close()/initAttempted guard as the always-reachable fixes and the context resolution as hardening is a fair way to put it. Two small asks: 1. Push the PR description and test Javadoc update you mentioned so they match that framing. 2. Ping me once that's up and I'll do a full pass on the remaining files. <!-- streview-comment:1023 --> -- 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]
