Conversation
Kubuxu
force-pushed
the
feat/pnet2
branch
2 times, most recently
from
a70886b to
74305d4
Compare
whyrusleeping
left a comment
whyrusleeping
left a comment
There was a problem hiding this comment.
Overall looks good to me. I'd like to see it squashed into two commits, the first just updating deps, and the second making all the code changes, then i'll review again.
| "github.com/ipfs/go-ipfs/repo/fsrepo" | ||
| iaddr "github.com/ipfs/go-ipfs/thirdparty/ipfsaddr" | ||
| pstore "gx/ipfs/QmQMQ2RUjnaEEX8ybmrhuFFGhAwPjyL1Eo6ZoJGD7aAccM/go-libp2p-peerstore" | ||
| swarm "gx/ipfs/QmY8hduizbuACvYmL4aZQbpFeKhEQJ1Nom2jY6kv6rL8Gf/go-libp2p-swarm" |
There was a problem hiding this comment.
lets keep all the dep updates in the same commit
Kubuxu
force-pushed
the
feat/pnet2
branch
2 times, most recently
from
2077a9d to
b217609
Compare
|
Done. |
whyrusleeping
left a comment
whyrusleeping
left a comment
There was a problem hiding this comment.
LGTM, Just one weird merge conflict thing with the dependency updating commit. Gotta make sure every commit at least builds
|
|
||
| tpt := makeSmuxTransport(mplex) | ||
|
|
||
| <<<<<<< HEAD |
There was a problem hiding this comment.
Sorry that I have missed it.
| case <-t.C: | ||
| if ph := n.PeerHost; ph != nil { | ||
| if len(ph.Network().Peers()) == 0 { | ||
| log.Warning("We are in private network and have no peers.") |
There was a problem hiding this comment.
Rather unfortunate that these warnings won't show up by default... I wonder if we should look at turning on warning messages by default?
There was a problem hiding this comment.
I think it might be good idea, there doesn't seem that much of a spam on warning level.
There was a problem hiding this comment.
I take it back, there is a lot of spam on global warning channel.
There was a problem hiding this comment.
Hrm... definitely out of scope for this PR, but we should look at reducing spam there and think about setting the default level to be warning
Kubuxu
force-pushed
the
feat/pnet2
branch
2 times, most recently
from
df2ae38 to
4acf869
Compare
|
Fixed the merge conflict and rebased. |
|
Coverage is a bit low, as the lack of peer warning isn't tested. There is no clean way to test it and as it is just a log I don't think it matters. |
|
|
||
| if node.PNetFingerpint != nil { | ||
| fmt.Println("Swarm is limited to private network of peers with the swarm key") | ||
| fmt.Printf("Swarm key fingerprint: %s\n", hex.EncodeToString(node.PNetFingerpint)) |
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
|
|
||
| // isolates the complex initialization steps | ||
| func constructPeerHost(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport) (p2phost.Host, error) { | ||
| func constructPeerHost(ctx context.Context, id peer.ID, ps pstore.Peerstore, bwr metrics.Reporter, fs []*net.IPNet, tpt smux.Transport, protec ipnet.Protector) (p2phost.Host, error) { |
There was a problem hiding this comment.
At some point (not now) I want to make this more simply composed
There was a problem hiding this comment.
Yeah, we should probably change it to be struct instead of function args.
|
LGTM, lets ship this in 0.4.7 |
2 participants
No description provided.