-
Notifications
You must be signed in to change notification settings - Fork 471
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
network: make GossipNode more independent from wsNetwork implementation #5634
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5634 +/- ##
==========================================
+ Coverage 54.95% 54.97% +0.02%
==========================================
Files 463 463
Lines 64476 64494 +18
==========================================
+ Hits 35432 35458 +26
+ Misses 26661 26656 -5
+ Partials 2383 2380 -3
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes makes sense for me although some implementation notes are below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general, I am pro moving interfaces out of implementation specific wsNetwork.go
file but doesn't need to happen here.
Summary
This removes some unused interface methods on GossipNode, and extracts some functionality from WebsocketNetwork into two new types (msgHandler, msgBroadcaster) to make them more available for re-use.
Test Plan
Existing tests should pass.