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.

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

Init client's params change to ...options #167

Merged
merged 6 commits into from
Dec 16, 2020
Merged

Init client's params change to ...options #167

merged 6 commits into from
Dec 16, 2020

Conversation

mark4z
Copy link
Contributor

@mark4z mark4z commented Dec 9, 2020

Fixes #154

@codecov-io
Copy link

codecov-io commented Dec 9, 2020

Codecov Report

Merging #167 (9ef6a82) into master (58a3b62) will increase coverage by 0.86%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
+ Coverage   57.80%   58.67%   +0.86%     
==========================================
  Files          20       22       +2     
  Lines        1384     1447      +63     
==========================================
+ Hits          800      849      +49     
- Misses        513      525      +12     
- Partials       71       73       +2     
Impacted Files Coverage Δ
common/constant/server_config_options.go 73.33% <73.33%> (ø)
common/constant/client_config_options.go 100.00% <100.00%> (ø)
clients/naming_client/host_reator.go 49.38% <0.00%> (-9.88%) ⬇️
clients/naming_client/push_receiver.go 29.16% <0.00%> (-4.17%) ⬇️
common/nacos_server/nacos_server.go 1.67% <0.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a3b62...9ef6a82. Read the comment docs.

@mark4z
Copy link
Contributor Author

mark4z commented Dec 10, 2020

@sanxun0325 主体部分已经完成了

Copy link
Member

@binbin0325 binbin0325 left a comment

Choose a reason for hiding this comment

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

增加使用options创建出来的serverConfig以及clientConfig 调用 CreateConfigClient的UT,也许会更好

common/constant/client_config_options.go Outdated Show resolved Hide resolved
common/constant/client_config_options_test.go Outdated Show resolved Hide resolved
common/constant/client_config_options_test.go Outdated Show resolved Hide resolved
@mark4z
Copy link
Contributor Author

mark4z commented Dec 11, 2020

增加使用options创建出来的serverConfig以及clientConfig 调用 CreateConfigClient的UT,也许会更好

现在搞定了

@mark4z mark4z closed this Dec 11, 2020
@mark4z mark4z reopened this Dec 11, 2020
@lzp0412
Copy link
Collaborator

lzp0412 commented Dec 13, 2020

@mark4z 可以在文档添加下新用法的使用说明,然后可以在example里增加下示例

@mark4z
Copy link
Contributor Author

mark4z commented Dec 14, 2020

@mark4z 可以在文档添加下新用法的使用说明,然后可以在example里增加下

是修改还是增加

@binbin0325
Copy link
Member

@mark4z 可以在文档添加下新用法的使用说明,然后可以在example里增加下

是修改还是增加

增加,可以体现出既支持options也支持原有方式 。

Copy link
Member

@binbin0325 binbin0325 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@lzp0412 lzp0412 left a comment

Choose a reason for hiding this comment

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

LGTM

@binbin0325 binbin0325 merged commit 0647923 into nacos-group:master Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Init client's params change to ...options will be better
4 participants