AlexStocks commented on code in PR #3232:
URL: https://github.com/apache/dubbo-go/pull/3232#discussion_r2902473095


##########
client/options.go:
##########
@@ -98,15 +93,8 @@ func (refOpts *ReferenceOptions) init(opts 
...ReferenceOption) error {
        }
 
        // init method
-       methods := refConf.MethodsConfig
-       if length := len(methods); length > 0 {
-               refOpts.methodsCompat = make([]*config.MethodConfig, length)
-               for i, method := range methods {
-                       refOpts.methodsCompat[i] = compatMethodConfig(method)
-                       if err := refOpts.methodsCompat[i].Init(); err != nil {
-                               return err
-                       }
-               }
+       if refConf.MethodsConfig == nil {
+               refConf.MethodsConfig = make([]*global.MethodConfig, 0)

Review Comment:
   原代码对每个 `MethodConfig` 调用 `config.MethodConfig.Init()`,执行 `defaults.Set` + 
TpsLimitStrategy/TpsLimitInterval/TpsLimitRate 合法性校验(负数检查、extension 
注册检查)。新代码把这些校验全部删掉,只初始化空 slice。
   
   用户配置了无效的 TpsLimitStrategy 或负数 TpsLimitRate,现在要等到运行时在 filter 层才崩溃,原本启动时就能 
fail-fast 的问题被延后了。
   
   建议:在 `global.MethodConfig` 中添加 `Validate()` 方法,并在 `ReferenceOptions.init()` 
中调用。



##########
client/options.go:
##########
@@ -447,14 +422,15 @@ func WithMeshProviderPort(port int) ReferenceOption {
        }
 }
 
-func WithMethod(opts ...config.MethodOption) ReferenceOption {
-       regOpts := config.NewMethodOptions(opts...)
-
+func WithMethod(method *global.MethodConfig) ReferenceOption {

Review Comment:
   这是破坏性 API 变更。原来是 functional options 风格(`WithMethod(config.WithName("foo"), 
config.WithRetries(3))`),现在变成直接传 struct 指针,外部调用方升级后直接编译失败。
   
   PR 描述没有标注 breaking change,也没有 migration guide 或 deprecation 期。
   
   建议:保留旧签名做 adapter 转换为新格式;或在 changelog 和 PR 说明中明确标注 breaking change,并同步处理 
`server/options.go` 中对应的 `WithMethod`。



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

Reply via email to