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


##########
config/consumer_config.go:
##########
@@ -185,7 +185,14 @@ func (cc *ConsumerConfig) Load() {
 
 // SetConsumerConfig sets consumerConfig by @c
 func SetConsumerConfig(c ConsumerConfig) {

Review Comment:
   SetConsumerConfig 里的 nil 分支在 rc == nil 时会丢失其他字段。如果 GetRootConfig() 还没被调用过(返回 
nil),这里直接 SetRootConfig(RootConfig{Consumer: &c}) 会创建一个只有 Consumer 的 
RootConfig,之前通过 Load() 设置的 Protocol、Application 等字段全部丢失。
   
   建议:如果 GetRootConfig() 返回 nil,直接用 NewRootConfigBuilder() 构建一个默认值,再设置 Consumer:
   ```go
   rc := GetRootConfig()
   if rc == nil {
       rc = NewRootConfigBuilder().Build()
   }
   next := *rc
   next.Consumer = &c
   SetRootConfig(next)
   ```



##########
config/root_config_test.go:
##########
@@ -90,3 +92,24 @@ func TestNewRootConfigBuilder(t *testing.T) {
        config := GetRootConfig()
        assert.Equal(t, rootConfig, config)
 }

Review Comment:
   TestRootConfigConcurrentSetAndGet 的 200 个并发 goroutine 数量在某些低端 CI 
环境(如树莓派或低配虚拟机)上可能会超时或触发 race detector。建议把数量降低到 50,或者加上 go test 的 -p 限制。
   
   另外,这个测试只验证了不 panic 和 GetRootConfig() 非 nil,建议加一个 assert 检查在所有 Set/Get 
完成后,GetRootConfig() 返回的是一个有效的 RootConfig(Application.Name 不为空)。



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