DanielLeens commented on PR #12015: URL: https://github.com/apache/seatunnel/pull/12015#issuecomment-5812758363
@luozihen thanks for pinning the F1-F8 checklist to concrete `path:line` locations for @SEZ9 - I independently spot-checked every citation against the current head (`00acc23a582d76a589c00df956dded4b6f21da94`) via `git show`/`git grep`, no trust-the-description shortcuts, and they all line up exactly: - F1: `ReadonlyConfig.java:76` is `toConfig()` (unchanged); `ConfigShadeUtils.java:224` is the JSON-syntax reparse inside `processConfig` (the shade-rebuild fix); `MultiTableFailureHelper.java:64` is `mergeOptions(...)`. - F2/F5: `JdbcSinkFactory.java:314` is `resolveMultiTablePrimaryKeys(...)`, `:344` is `toCompiledPatternMap(...)` (the `LinkedHashMap` that guarantees first-match-wins). - F3: `:282` is `validatePrimaryKeyColumns(...)`. - F4: `:416` is `expandPrimaryKeyPlaceholder(...)`. - F6: `:361` is `compilePattern(...)`. - F8: `JdbcSinkOptions.java:112` is the `MULTI_TABLE_CONFIG` option declaration - fully snake_case as claimed. - F7: both `ReadableConfigTest` and `JdbcMysqlMultipleTablesIT` exist at the paths given in the PR description and cover what's claimed. Since the head SHA hasn't changed since my last full re-review and approval, I don't have anything new to add here beyond confirming these line citations are accurate - this is just closing out @SEZ9's verification request, not a functional change. My "Ready to merge" conclusion from the last round still stands; still waiting on a green CI run and a write-capable maintainer for the final merge action. -- 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]
