basichost: allow overriding Addrs() #196
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
whyrusleeping
suggested changes
May 29, 2017
hsanjuan
reviewed
May 29, 2017
p2p/host/basic/basic_host.go
Outdated
| @@ -23,6 +23,8 @@ var log = logging.Logger("basichost") | |||
|
|
|||
| var NegotiateTimeout = time.Second * 60 | |||
|
|
|||
| type AddrsFactory func([]ma.Multiaddr) []ma.Multiaddr | |||
There was a problem hiding this comment.
Document this type please. Why it is here? What is it used for and maybe a practical example of how it can be used.
p2p/host/basic/basic_host.go
Outdated
| @@ -23,6 +23,8 @@ var log = logging.Logger("basichost") | |||
|
|
|||
| var NegotiateTimeout = time.Second * 60 | |||
|
|
|||
| type AddrsFactory func([]ma.Multiaddr) []ma.Multiaddr | |||
|
|
|||
| // Option is a type used to pass in options to the host. | |||
There was a problem hiding this comment.
great opportunity to also improve Option documentations :)
There was a problem hiding this comment.
I have a bit of work-in-progress code to refactor construction of BasicHost and Swarm, the Option type will be gone afterwards. Separate PR though :)
There was a problem hiding this comment.
I counted and there's at least 4 different ways of passing in various options to host and swarm, including global variables :) -- it's in bad need of an options struct.
ghost
force-pushed
the
feat/addresses-announce
branch
from
4067302 to
6cbb939
Compare
|
Okay here we go |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is for ipfs/kubo#3613. I decided to cheat and just sidestep the whole passing-options-through-n-layers-of-node-initialization. The calling code will simply pass a closure as the
AddrsFactoryoption which applies the announced/noAnnounced addrs if neccessary.(I'm open to better names than AddrsFactory.)
TODO: