SEZ9 commented on PR #12129: URL: https://github.com/apache/seatunnel/pull/12129#issuecomment-5738848181
Thanks for the detailed recap against `408f1e0` — that covers all six points from my earlier review, and the described changes sound like exactly what I was after: 1. Null `JobStatus` in `cancelJob()`/`stopJob()` failing fast before the `contains(...)` check, with an inline comment noting the dependency on that call ordering. 2. Javadoc explaining why `SCHEDULED` is excluded from `NOT_YET_STARTED_STATES`. 3. The pinning test asserting the length plus per-index ordinals with a message naming the drifted constant. 4. Test Javadoc scoped to same-build protection only, calling out the mixed-version `ArrayIndexOutOfBoundsException` risk. 5. `NOT_YET_STARTED_STATES` wrapped in `Collections.unmodifiableSet`. 6. Wording cleanup in the same Javadoc rewrite. I haven't yet re-read the diff at `408f1e0` myself, so I'll do that before marking these resolved. If you can point me at the specific hunks for items 1 and 3 (the null-handling comment and the pinning-test assertions), that would speed it up. No further code asks from me beyond confirming those. <!-- streview-comment:1168 --> -- 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]
