-
Notifications
You must be signed in to change notification settings - Fork 976
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
fix(kad): remove default IPFS DHT protocol name #5015
Conversation
@@ -181,10 +181,11 @@ pub struct Config { | |||
provider_publication_interval: Option<Duration>, | |||
kbucket_inserts: BucketInserts, | |||
caching: Caching, | |||
protocol_name: String, |
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.
Instead of having a new field, we can make use of the protocol_config
by using ProtocolConfig::set_protocol_names
.
The current default is set here: https://github.com/robin-rrt/rust-libp2p/blob/c29a844506fe9eb8edb0526d5a4f2c1f2296b129/protocols/kad/src/protocol.rs#L167 That is what needs to be changed :) |
Just to understand. Do we want the Default implementation gone completely? And have a |
Yes, I think that is what @mxinden meant in #5006. Now, I am wondering if we couldn't:
This might allow us to ship this as a backwards-compatible change. I haven't tested this though. We need to make sure it doesn't cause compile errors for users because of the We can't mark the actual |
Closing in favor of #5122. |
Description
Fix #5006
Notes & open questions
Added a parameter for protocol name.
Change checklist