Be extra explicit on DHT configuration in CONFIGURATION.md #951
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This weekend I hooked up DHT in my js-libp2p app. It did not work, because I had not set dht.enable:true in the configuration on the client side. I spent an embarrassing amount of time trying to fix this and wound up filing a bug. Oops.
However, one thing occurs to me. I remember reading CONFIGURATION.md at some point while setting my app up, and I think I didn't realize the configuration was necessary because of the line "enabled by default" immediately after. There are two "enabled:true" lines immediately next to each other, one is labeled "enabled by default" (and is), the other is disabled by default. I think I saw "enabled:true // by default" and thought ah! So I don't need to set it! and totally blanked that there was two lines away a second "enabled:true" with no such comment.
The fact I got confused is 100% my fault. But if CONFIGURATION.md confused me, it might confuse other people, so here is a patch that adds a comment to the other line to make it clear that the second enabled:true is NOT true by default and in fact is very important.