-
Notifications
You must be signed in to change notification settings - Fork 26
Conversation
maddrParsers[maname] = mp | ||
} | ||
|
||
func ParseTcpNetAddr(a net.Addr) (ma.Multiaddr, error) { |
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.
Do the Parse*
methods need to be public? If so, tests would be awesome!
This gets closer to the modularity described in #7 but doesnt yet go all the way due to the complexity of refactoring every package that uses this |
addrParsers = make(map[string]AddrParser) | ||
maddrParsers = make(map[string]MaddrParser) | ||
|
||
RegisterAddressType(tcpAddrSpec) |
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.
Would it make more sense for the default specs to live in here rather than convert.go
?
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.
hrm... you might be right there. code organization is hard
🐴 LGTM |
) | ||
|
||
type AddrParser func(a net.Addr) (ma.Multiaddr, error) | ||
type MaddrParser func(ma ma.Multiaddr) (net.Addr, error) |
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 these two functions, which are named a bit confusingly, either:
type NetAddr2MAddr func(a net.Addr) (ma.Multiaddr, error)
type MAddr2NetAddr func(ma ma.Multiaddr) (net.Addr, error)
// or
type FromNetAddr func(a net.Addr) (ma.Multiaddr, error)
type ToNetAddr func(ma ma.Multiaddr) (net.Addr, error)
// or even
type NetCodec struct {
FromNet func(a net.Addr) (ma.Multiaddr, error)
ToNet func(a net.Addr) (ma.Multiaddr, error)
}
var TCPNetCodec = NetCodec{}
TCPNetCodec.FromNet = func ...
TCPNetCodec.ToNet = func ...
@jbenet addressed your feedback |
thanks for addressing the comments ❤️ ❤️ |
multiaddr net needed some more modularity. I've started on this here.