SEZ9 commented on PR #12491: URL: https://github.com/apache/seatunnel/pull/12491#issuecomment-5923588586
Thanks for the update, @NganWave, and for the nine commits between `f83f5a70` and `1c632c25`. On `ConfEncryptCommand`: agreed. If `ConfigShadeUtils.encryptConfig()` is not behaving as expected for sensitive fields, that deserves its own investigation rather than being folded into this change. Keeping Issue #10 open as a follow-up is fine with me; please just note the deferral in the PR description so it is visible to anyone landing here later. The code at head `1c632c25` looks in good shape, in particular `ParameterSplitter` now throwing `ParameterException` with `CommandLineUtils` turning it into a clean usage error, and `processVariable` no longer re-typing substituted strings (the `ec82676a` -> `f83f5a70` change). Two things still block this round: 1. **Documentation.** Please push the `-i` docs you have locally: the JSON map/list syntax, quoting examples, the duplicate-key rejection, and the plugin-node restriction that the negative test covers. A separate commit is fine; I'll review it independently of the code. 2. **Merge conflicts with `dev`.** Please rebase/merge and resolve them so the branch is mergeable. Once those are in I'll do a final pass over the full diff. Appreciate the careful, incremental approach here; it has made each round easy to verify. <!-- streview-comment:1440 --> -- 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]
