Skip to content
New issue

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

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

Already on GitHub? # to your account

Mod:cancel listener dir when zkpath end of providers/ & consumers/ #359

Merged
merged 3 commits into from
Feb 19, 2020

Conversation

hxmhlt
Copy link
Contributor

@hxmhlt hxmhlt commented Feb 16, 2020

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@hxmhlt hxmhlt changed the title Mod zk listen Mod:cancel listener dir when zkpath end of providers/ & consumers/ Feb 16, 2020
@codecov-io
Copy link

codecov-io commented Feb 16, 2020

Codecov Report

Merging #359 into develop will decrease coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #359      +/-   ##
===========================================
- Coverage    66.74%   66.57%   -0.17%     
===========================================
  Files          150      150              
  Lines         7936     7938       +2     
===========================================
- Hits          5297     5285      -12     
- Misses        2137     2149      +12     
- Partials       502      504       +2
Impacted Files Coverage Δ
remoting/zookeeper/listener.go 52.57% <100%> (+0.54%) ⬆️
config_center/nacos/facade.go 35.29% <0%> (-29.42%) ⬇️
cluster/cluster_impl/failback_cluster_invoker.go 78.49% <0%> (-2.16%) ⬇️
config_center/nacos/client.go 55.55% <0%> (-1.71%) ⬇️

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 d0d3ac1...e501e80. Read the comment docs.

Copy link
Member

@pantianying pantianying left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -195,20 +195,20 @@ func GetApplicationConfig() *ApplicationConfig {

// GetProviderConfig find the provider config
// if not found, create new one
func GetProviderConfig() ProviderConfig {
func GetProviderConfig() *ProviderConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

are u sure u need to change the value of the ProviderConfig by its pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Be same as other func. GetMetricConfig / GetApplicationConfig

@AlexStocks
Copy link
Contributor

LGTM

Copy link
Member

@flycash flycash left a comment

Choose a reason for hiding this comment

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

LGTM

@pantianying pantianying merged commit 2d4022d into apache:develop Feb 19, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants