DanielLeens commented on PR #12443: URL: https://github.com/apache/seatunnel/pull/12443#issuecomment-6383297441
Thanks @hutiefang76 for the location-by-location confirmation, and @SEZ9 for asking for it. I checked the CI side directly: the required `Build` check on this PR (https://github.com/apache/seatunnel/runs/119994181527) completed with `success` for head `509fddabe2ca6850d90a48b260df914d6a6704c3`, and the fork run behind it (https://github.com/hutiefang76/seatunnel/actions/runs/39774794745) finished `success` with 85 jobs passed, 11 skipped and none failed, so the REST ITs and the earlier engine-suite lanes are covered on this exact head. `labeler` and `Notify test workflow` are green too. That clears the CI gate I listed as the only blocker in my last review. On the two asks from @SEZ9, the pointers line up with what I verified earlier on the same head: the guard wraps the map at `ReadonlyConfig.java:79`, `ConfigBuilder.java:111, 243, 264, 278, 292` and `ConfigShadeUtils.java:229`, and the metadata/application-config sites stay in the follow-up #12944. For the naming point, the test method is already `testWhitespacePaddedKeysTrimmedUnlessGuardQuotesWholeKey`; only the value label `"dotted"` for the `" job.mode "` key (`ConfigBuilderTest.java:79`) still carries the old wording, which is cosmetic and non-blocking. The head has not changed, so this is a status follow-up rather than a new full review. From my side there is no remaining source-level or CI blocker: I consider this ready to merge, with the remaining `parseMap` sites as a tracked follow-up. What is left is @SEZ9's final pass and a write-capable committer's approval (`reviewDecision` is still `REVIEW_REQUIRED`). Thanks again! -- 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]
