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.

Sign up for GitHub

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

Jump to bottom

refactor: Simplify network protocols #1968

Merged
merged 3 commits into from
Apr 16, 2020
Merged

refactor: Simplify network protocols #1968

merged 3 commits into from
Apr 16, 2020

Conversation

TheWaWaR
Copy link
Contributor

  • Move p2p protocols to network module
  • Change some discovery log message's log level from debug to trace
  • Remove DiscoveryService struct

@TheWaWaR TheWaWaR requested review from a team and zhangsoledad March 17, 2020 17:38
@doitian doitian added the s:waiting-on-reviewers Status: Waiting for Review label Mar 19, 2020
@TheWaWaR TheWaWaR force-pushed the simplify-network-protocols branch 2 times, most recently from 4165edc to 9d56ecf Compare March 19, 2020 13:49
@yangby-cryptape
Copy link
Collaborator

Reuse a big crate is not easy.
If anyone want to use these protocols, they have to import the whole network crate (why they have to use sentry) or write other implements?

If we move all protocols away from network, people can use there protocols in their own way.
We communicate through protocols, not implements.

Since I have already write another implement of these protocols.
I don't oppose this change, just express my opinion.

@doitian doitian requested a review from driftluo March 26, 2020 03:46
@driftluo
Copy link
Collaborator

please rebase @TheWaWaR

@TheWaWaR TheWaWaR force-pushed the simplify-network-protocols branch 2 times, most recently from 5cea765 to acfb21c Compare March 31, 2020 15:37
quake
quake previously approved these changes Apr 15, 2020
View reviewed changes
@TheWaWaR
Copy link
Contributor Author

bors r+

@nervos-bot nervos-bot bot added the s:ready-to-merge Status: Waiting to be merged. label Apr 16, 2020
@bors
Copy link
Contributor

bors bot commented Apr 16, 2020

Build succeeded:

@bors bors bot merged commit 9c94722 into nervosnetwork:develop Apr 16, 2020
@TheWaWaR TheWaWaR deleted the simplify-network-protocols branch June 10, 2022 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driftluo driftluo driftluo approved these changes

@quake quake quake approved these changes

@zhangsoledad zhangsoledad Awaiting requested review from zhangsoledad

Assignees
No one assigned
Labels
s:ready-to-merge Status: Waiting to be merged. s:waiting-on-reviewers Status: Waiting for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@TheWaWaR @yangby-cryptape @driftluo @quake @doitian