DanielLeens commented on PR #10678: URL: https://github.com/apache/seatunnel/pull/10678#issuecomment-5322378361
@SEZ9 Happy to summarize from my side, since I reviewed and approved this PR on 2026-08-15 against the current head (`f111e48`). **Status: no blocking items remain.** Every substantive issue raised across the review history — the original `JarFileFactory` cross-job JAR-sharing hazard, the incomplete JDK 9+ `--add-opens` guidance, the JDK 8 global `useCaches` mutation on the default path, the static-flag cross-instance hazard, the JDK-8-fatal docs instruction, and the unreachable sidebar page — is confirmed fixed on the current head. CI (`Build`) is green there too. There are two non-blocking recommended items from my last review, neither of which needs to gate a merge: 1. (Medium) The `SEATUNNEL_CLASSLOADER_DEEP_CLEAN` environment-variable fallback (added alongside the existing `-D` system-property form) isn't mentioned in `docs/en` or `docs/zh`, which currently document only the `-D` flag. 2. (Medium, carryover) The three `ClassLoaderServiceTest` cases that mutate JVM-global `URLConnection` cache state rely on `@TestMethodOrder` for isolation rather than `@Isolated`/a separate Surefire fork, so a future test added elsewhere in the module could someday make that regression guard silently skip instead of fail. Both are cheap follow-ups. Given @knight6236's note above about stepping back from further iteration on this PR, I'd say it's reasonable for a maintainer to merge as-is and track these two as a small follow-up (a quick patch from a committer, or a tracked issue) rather than blocking on them. I don't have merge authority myself, so that call is yours/the maintainers' to make — just wanted to make sure the current state was on the record clearly. -- 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]
