SEZ9 commented on issue #13079:
URL: https://github.com/apache/seatunnel/issues/13079#issuecomment-6622779815

   Thanks @sharpca — `cabfb80d` addresses both test points: one parameterized 
invocation per converter with the dialect name carried into the assertion 
message, and the locale capture/restore moved into the before/after hooks so 
the restore is structural.
   
   Two things remain before review continues on #13083 itself:
   
   1. **CI has not run on the fork yet.** As described above, `action_required` 
on the apache side is only the `Label-when-reviewed` workflow, which does not 
build or test anything, and the `Build` check on #13083 cannot pick anything up 
until a workflow run exists in `sharpca/seatunnel`. There is no maintainer 
approval gate to clear. Please enable Actions in your fork and run the 
**Build** workflow against the PR branch (or push an empty commit); the PR 
check should pick it up within about 15 minutes. The same applies to #13081. 
This matters most for the integration case added in `704af8840`, which you 
noted has not been compiled or run locally.
   
   2. **PR description vs. actual scope.** The `Locale.ROOT` change touches 
more files than this thread lists (for example `Gbase8aTypeMapper`, 
`HiveTypeMapper`, `PrestoTypeMapper`, `PhoenixTypeConverter`, 
`SnowflakeTypeConverter`, the DuckDB/Redshift/Xugu/YashanDB catalogs, 
`DamengDataTypeConvertor`, `OceanBaseOracleCreateTableSqlBuilder`, and the new 
`JdbcMysqlSplitIT` case). Please list every touched file in the PR description 
with the call site and why it is locale sensitive, in the same style as the 
table you posted above, so the review can be checked against the full set of 
call sites.
   
   The boundaries with #13063 (`JdbcDialect` / `CatalogUtils`) and #13076 / 
#13077 (MySQL / OceanBase / SQL Server converters) look clean on the main 
sources — nothing needed there.
   
   Once the fork build has executed and the description is complete, I will 
continue on #13083.
   
   <!-- streview-comment:2272 -->


-- 
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]

Reply via email to