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

feat: support grpc json protocol #582

Merged
merged 6 commits into from
Jun 14, 2020

Conversation

relaxedCat
Copy link
Contributor

Fixes #437 comment

@AlexStocks
Copy link
Contributor

@relaxedCat pls add comment for the public structs/funcs.

@AlexStocks AlexStocks requested review from zouyx, fangyincheng and AlexStocks and removed request for zouyx June 2, 2020 15:57
@zouyx
Copy link
Member

zouyx commented Jun 3, 2020

Travis still failing

protocol/grpc/codec.go Outdated Show resolved Hide resolved
@relaxedCat relaxedCat force-pushed the feat-grpc-json branch 3 times, most recently from 97547bd to 7530503 Compare June 7, 2020 12:41
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2020

Codecov Report

Merging #582 into develop will decrease coverage by 0.08%.
The diff coverage is 46.26%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #582      +/-   ##
===========================================
- Coverage    67.20%   67.12%   -0.09%     
===========================================
  Files          174      190      +16     
  Lines         9261     9918     +657     
===========================================
+ Hits          6224     6657     +433     
- Misses        2432     2605     +173     
- Partials       605      656      +51     
Impacted Files Coverage Δ
protocol/grpc/codec.go 40.90% <40.90%> (ø)
protocol/grpc/client.go 61.70% <48.38%> (-27.78%) ⬇️
protocol/grpc/config.go 50.00% <50.00%> (ø)
registry/kubernetes/registry.go 57.69% <0.00%> (-13.74%) ⬇️
registry/zookeeper/registry.go 46.15% <0.00%> (-9.30%) ⬇️
cluster/directory/base_directory.go 56.81% <0.00%> (-9.10%) ⬇️
protocol/dubbo/listener.go 57.52% <0.00%> (-5.38%) ⬇️
protocol/dubbo/pool.go 76.81% <0.00%> (-4.47%) ⬇️
config_center/nacos/impl.go 71.08% <0.00%> (-4.31%) ⬇️
config_center/apollo/impl.go 82.71% <0.00%> (-4.30%) ⬇️
... and 106 more

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 8a3c02c...45a983e. Read the comment docs.

@AlexStocks
Copy link
Contributor

Pls add comments for funcs/stucts.

@relaxedCat
Copy link
Contributor Author

Pls add comments for funcs/stucts.

done.

Copy link
Contributor

@fangyincheng fangyincheng left a comment

Choose a reason for hiding this comment

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

LGTM

// load clientconfig from consumer_config
// default use grpc
defaultClientConfig := GetDefaultClientConfig()
clientConf = &defaultClientConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

the above two line codes is very strange. why now define a NewDefaultClientConfig() *ClientConfig?

Copy link
Contributor Author

@relaxedCat relaxedCat Jun 14, 2020

Choose a reason for hiding this comment

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

ok,use defer delay check instead

@AlexStocks AlexStocks merged commit b9f71f7 into apache:develop Jun 14, 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.

6 participants