DanielLeens commented on PR #12352: URL: https://github.com/apache/seatunnel/pull/12352#issuecomment-5726031640
Thanks for the quick turnaround, @Nikk8091! Confirmed on my side too — this PR only attaches `notBlank` value constraints to options that were already required in `SlsSourceFactory`/`SlsSinkFactory`; it doesn't add any new required option or change any option's presence/type, so my earlier read stands. The rebase onto the latest `dev` looks clean, and the whitespace-handling behavior you describe (surrounding-space values still accepted, blank/whitespace-only rejected) matches what I traced through `ConditionEvaluators.NOT_BLANK` during review. Good to see `mvnw -pl seatunnel-connectors-v2/connector-sls -DskipITs verify` passing locally as well. My earlier review was an APPROVE with only non-blocking suggestions (strengthening the test's exception-message assertion, a possible follow-up on trimming/documenting whitespace behavior, and an optional E2E negative case) — none of those are blockers, so this remains ready-to-merge from my side. The `Build` check on this fresh commit is still finishing up, and the PR currently shows `REVIEW_REQUIRED` (likely because the new push reset review state per branch protection) — once CI is green this should just need another maintainer's approval to clear that gate. Thanks again for addressing the follow-ups so thoroughly! -- 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]
