SEZ9 commented on issue #12617: URL: https://github.com/apache/seatunnel/issues/12617#issuecomment-5988185870
Thanks @SEPURI-SAI-KRISHNA, this is in good shape. **#12618** — the four tr-TR tests following the save/set/restore-in-`finally` pattern from #12495 are exactly what I wanted, and the check that reverting each `Locale.ROOT` fails only its own test is the right way to prove they guard a regression rather than just pass. Please add a short note in the PR description that `BuiltinFunctions:54` (and the other hardening-only sites) are consistency changes rather than reachable defects, so reviewers read the diff with the same five-reachable / six-hardening split you measured here. One thing to sort out before I can take it: the aggregate Build check on #12618 is currently red. Can you take a look and either fix it or tell me whether it is unrelated to your change? **`FieldRenameTransform:167/170` / `TableRenameTransform:153/155`** — agreed, out of scope. Those apply user-requested casing to data, which is a different contract from matching an internal token, and changing it needs its own discussion. **Lint rule** — yes, please open the tracking issue now, but keep the guard out of #12618. For the follow-up issue I'd like the scope to: 1. enumerate the internal-token matching sites that require a fixed locale, 2. explicitly preserve the intentional user-data casing operations (the two rename transforms above), 3. define how an intentional exception is documented so the guard does not become noise. Please hold off on adding a Checkstyle/forbidden-apis dependency or a broad regex gate until that scope and the existing baseline have been reviewed, and start the implementation PR only after #12495 and #12618 have landed so the rule is evaluated against the post-fix module instead of failing on sites those two PRs already fix. Summary of asks: (a) get #12618's Build green or explain the failure, (b) add the hardening-vs-reachable note to the PR description, (c) open the lint-rule tracking issue with the scope above and link it back here. <!-- streview-comment:1530 --> -- 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]
