Skip to content

feat: support grpc json protocol #582

Merged
AlexStocks merged 6 commits intoapache:developfrom
relaxedCat:feat-grpc-json
Jun 14, 2020
Merged

feat: support grpc json protocol #582
AlexStocks merged 6 commits intoapache:developfrom
relaxedCat:feat-grpc-json

Conversation

@relaxedCat
Copy link
Copy Markdown
Contributor

Fixes #437 comment

@AlexStocks
Copy link
Copy Markdown
Contributor

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

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

zouyx commented Jun 3, 2020

Travis still failing

@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
Copy Markdown

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
Copy Markdown
Contributor

Pls add comments for funcs/stucts.

@relaxedCat
Copy link
Copy Markdown
Contributor Author

Pls add comments for funcs/stucts.

done.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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