Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

change method WithClientCheck to WithClientNoCheck #2680

Merged
merged 1 commit into from
May 20, 2024
Merged

change method WithClientCheck to WithClientNoCheck #2680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions client/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,9 @@ func (cliOpts *ClientOptions) init(opts ...ClientOption) error {

type ClientOption func(*ClientOptions)

func WithClientCheck() ClientOption {
func WithClientNoCheck() ClientOption {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

与其改名字，还不如传一个 check bool 参数进来

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这套 options 的 api 整体风格差不多就是枚举配置项，传参有点风格不一致

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这套 options 的 api 整体风格差不多就是枚举配置项，传参有点风格不一致

这个接口改动后，关键是会影响外部用户使用，所以我还是建议不要改名字，可以在注释上把函数的含义解释清楚。对外接口，不要轻易改动。

OR

把这个函数标记为 deprecated，重新实现一个新函数 WithClientNoCheck

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是这样的雨哥，这个 API 目前只发布了一个 rc 版本，使用的人非常少，API 还处于一个不太稳定的状态，而且其实 withCheck 方法用不到，因为默认值就是 true， withCheck 也是设置为true，所以我觉得留着没啥意义

return func(opts *ClientOptions) {
opts.Consumer.Check = true
opts.Consumer.Check = false
}
}

Expand Down
4 changes: 2 additions & 2 deletions client/options_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ func TestWithClientCheck(t *testing.T) {
{
desc: "config check",
opts: []ClientOption{
WithClientCheck(),
WithClientNoCheck(),
},
verify: func(t *testing.T, cli *Client, err error) {
assert.Nil(t, err)
assert.Equal(t, true, cli.cliOpts.Consumer.Check)
assert.Equal(t, false, cli.cliOpts.Consumer.Check)
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions dubbo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (ins *Instance) NewClient(opts ...client.ClientOption) (*client.Client, err
otelCfg := ins.insOpts.CloneOtel()

if conCfg != nil {
if conCfg.Check {
cliOpts = append(cliOpts, client.WithClientCheck())
if !conCfg.Check {
cliOpts = append(cliOpts, client.WithClientNoCheck())
}
// these options come from Consumer and Root.
// for dubbo-go developers, referring config/ConsumerConfig.Init and config/ReferenceConfig
Expand Down
Loading