DanielLeens commented on PR #12578: URL: https://github.com/apache/seatunnel/pull/12578#issuecomment-6097848912
Quick follow-up from my side, since the thread has moved on since my last review and a couple of facts have changed. Thanks @hutiefang76 for the very clear answers above, and @SEZ9 for the careful tracking. 1. CI status has changed since my review. My earlier note said the fork run had two failing jobs (`all-connectors-it-7 (8)` and `paimon-connector-it (8)`) and suggested retriggering only those. At the current head `ec14f3a3a421` the fork `Build` run (https://github.com/hutiefang76/seatunnel/actions/runs/37046261613) is now completed with a success conclusion, and the apache-side `Build` pointer check is green as well. So that CI remark in my review is out of date, and there is nothing left to retrigger. The branch is behind `dev` but GitHub reports no conflict, so a sync is not needed on its own. 2. On @SEZ9's last comment, which lists F1 and F2 as still open: I checked both against the source at `ec14f3a3a421`, and I believe they are already addressed in the current head (the earlier head `81dd83e9f905` that comment was based on had not yet included them). - F1 (hot path): `seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/SeaTunnelRow.java:58-60` now reads `size` once and only writes `size = 0` when it is non-zero, so a freshly assembled row pays no volatile write. - F2 (contract and Javadoc): `setField` (`SeaTunnelRow.java:52-55`), `getFields` (`:103`) and both `getBytesSize` overloads carry Javadoc describing the invalidation and its limits, and the paragraph now lives in the shared "Row Data Contract" section of `docs/en/architecture/core-api-design.md` and the matching ZH page. @SEZ9, could you take another look at `ec14f3a3a421` when you have a moment? I think the remaining items you listed are covered, and the three quoted test snippets match the file at that head. My conclusion is unchanged: no blocking issues from my side, ready to merge. As before, I only have read-only permissions here, so the final approval step still needs a maintainer with write access. -- 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]
