Skip to content
This repository has been archived by the owner on Oct 5, 2021. It is now read-only.

make multiaddr-net more pluggable #15

Merged
merged 5 commits into from
May 16, 2016
Merged

make multiaddr-net more pluggable #15

merged 5 commits into from
May 16, 2016

Conversation

whyrusleeping
Copy link
Member

multiaddr net needed some more modularity. I've started on this here.

@whyrusleeping
Copy link
Member Author

@lgierth @noffle @jbenet could I get some CR here?

maddrParsers[maname] = mp
}

func ParseTcpNetAddr(a net.Addr) (ma.Multiaddr, error) {
Copy link

@hackergrrl hackergrrl May 3, 2016

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!

@ghost ghost mentioned this pull request May 4, 2016
@whyrusleeping
Copy link
Member Author

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)

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?

Copy link
Member Author

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

@hackergrrl
Copy link

🐴 LGTM

)

type AddrParser func(a net.Addr) (ma.Multiaddr, error)
type MaddrParser func(ma ma.Multiaddr) (net.Addr, error)
Copy link
Member

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 ...

@whyrusleeping
Copy link
Member Author

@jbenet addressed your feedback

@jbenet jbenet merged commit f355062 into master May 16, 2016
@jbenet jbenet deleted the feat/pluggable branch May 16, 2016 19:12
@jbenet
Copy link
Member

jbenet commented May 16, 2016

thanks for addressing the comments ❤️ ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants