yunfan24 commented on PR #1120: URL: https://github.com/apache/incubator-seata-go/pull/1120#issuecomment-5108342340
@slievrly 1. (Blocking) go 1.20 → 1.25 破坏性变更未标注 已修改。 在 changes/dev.md 和 changes/dev_zh.md 中新增 breaking change 分类，显著标注 Go 版本升级和 parser 替换。上游 github.com/pingcap/tidb/pkg/parser 的 go.mod 要求 go 1.25，floor 无法降低。 2. (Blocking) ~90 文件范围过大，建议拆分 PR 未修改。 文件数量多的原因： 1. import 路径替换：github.com/arana-db/parser → github.com/pingcap/tidb/pkg/parser 涉及约 60 个文件，这是依赖替换的固有范围，无法避免 2. lint 修复：Go 版本从 1.20 升至 1.25 后，新版本 linter 检查更严格，带来的修复都是简单重复性修改（如移除冗余 nil 检查、简化 switch 类型断言、fmt.Sprintf → fmt.Fprintf 等），review 难度不大 3. 拆分会导致 CI 失败：格式化修复依赖 Go 版本升级，parser 迁移依赖 lint 修复通过 CI 检查，拆分后单独的 PR 无法通过 CI 3. (Blocking risk) test_driver 进了生产代码 未修改。 master 分支上 base_executor.go 和 basic_undo_log_builder.go 已经使用了 github.com/arana-db/parser/test_driver。迁移只是从 arana-db/parser/test_driver 换成 pingcap/tidb/pkg/parser/test_driver，并非本次新引入的问题。 4. (Blocking risk) param marker 顺序等价性缺专门测试 已修改。 在 parse_factory_test.go 中新增 TestAssignParamMarkerOrdersComplex English Version: --- ### 1. (Blocking) Go 1.20 → 1.25 is a breaking change for downstream **Addressed.** A new **Breaking Changes** section has been added to both `changes/dev.md` and `changes/dev_zh.md`, explicitly highlighting the Go version upgrade and the parser replacement. Regarding the minimum Go version, the upstream module `github.com/pingcap/tidb/pkg/parser` declares **Go 1.25** in its `go.mod`. Therefore, the minimum Go version for this PR cannot be lowered without changing the upstream dependency. --- ### 2. (Blocking) ~90-file scope, most unrelated to the parser **Not addressed.** The large number of modified files is unavoidable for the following reasons: 1. **Import path replacement.** Replacing `github.com/arana-db/parser` with `github.com/pingcap/tidb/pkg/parser` affects approximately 60 files. This is an inherent consequence of the dependency migration and cannot reasonably be reduced. 2. **Lint fixes after the Go upgrade.** Upgrading from Go 1.20 to Go 1.25 introduces stricter linting rules. The resulting changes are straightforward mechanical fixes, such as removing redundant `nil` checks, simplifying type-switch assertions, and replacing `fmt.Sprintf` with `fmt.Fprintf`. These changes are repetitive and should not significantly increase the review effort. 3. **Splitting would break CI.** The formatting and lint fixes depend on the Go version upgrade, while the parser migration depends on those fixes to satisfy the CI checks. Splitting them into separate PRs would leave the individual PRs unable to pass CI. --- ### 3. (Blocking risk) `test_driver` ends up in production code **Not addressed.** The current `master` branch already imports `github.com/arana-db/parser/test_driver` in `base_executor.go` and `basic_undo_log_builder.go`. This migration only replaces `github.com/arana-db/parser/test_driver` with `github.com/pingcap/tidb/pkg/parser/test_driver`; it does **not** introduce `test_driver` into production code for the first time. This PR preserves the existing design rather than changing it. --- ### 4. (Blocking risk) No dedicated param-marker order equivalence test **Addressed.** A dedicated test, `TestAssignParamMarkerOrdersComplex`, has been added to `parse_factory_test.go` to verify that `assignParamMarkerOrders` correctly reconstructs parameter marker ordering in complex SQL statements. This specifically validates the ordering logic introduced by the migration and helps ensure consistent behavior for undo-log generation and lock-key construction. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
