issue#325 #354
issue#325 #354
Conversation
p2p/service/service.go
Outdated
| @@ -14,7 +14,7 @@ type Message interface { | |||
| // Service is an interface that represents a networking service (ideally p2p) that we can use to send messages or listen to incoming messages | |||
| type Service interface { | |||
| Start() error | |||
| RegisterProtocol(protocol string) chan Message | |||
| RegisterProtocol(protocol string, bufferSize int) chan Message | |||
There was a problem hiding this comment.
i would rather you pass the chan into RegisterProtocol
all places using RegisterProtocol should be changed accordingly
There was a problem hiding this comment.
I didn't get that, pass the chan into RegisterProtocol()
Did you mean like changing RegisterProtocol signature to accept chan as parameter?
Something like RegisterProtocol(name, chan Message)
There was a problem hiding this comment.
hi @adigunhammedolalekan are you planning to fix this ?
There was a problem hiding this comment.
@adigunhammedolalekan hi, nice work with the pr but you left this part out
can you fix this so we can merge ?
There was a problem hiding this comment.
Yes! I'll really love to, but can you please provide a little bit more details? I'll really appreciate that.
There was a problem hiding this comment.
I didn't get that, pass the chan into RegisterProtocol()
Did you mean like changing RegisterProtocol signature to accept chan as parameter?
Something likeRegisterProtocol(name, chan Message)
you need to change RegisterProtocol like we discussed and refactor the places it is used so the tests still pass.
so basically the only thing to do is change RegisterProtocol(protocol string, bufferSize int) chan Message to RegisterProtocol(protocol string, chan Message)
of course that means that you will have to create and pass the chan when calling RegisterProtocol.
|
hi @adigunhammedolalekan thanks for the pull request, looks good! |
|
Thanks @almogdepaz I'll fix that ASAP |
|
Hi @adigunhammedolalekan |
|
Hi, @antonlerner i have been trying to fix the test issue, i will fix it as soon as possible. Thanks |
| @@ -191,7 +192,7 @@ func (sn *Node) SubscribePeerEvents() (chan crypto.PublicKey, chan crypto.Public | |||
|
|
|||
| // RegisterProtocol creates and returns a channel for a given protocol. | |||
| func (sn *Node) RegisterProtocol(protocol string) chan Message { | |||
| c := make(chan Message) | |||
| c := make(chan Message, config.ConfigValues.BufferSize) | |||
There was a problem hiding this comment.
you need to change RegisterProtocol like we discussed and refactor the places it is used so the tests still pass.
so basically the only thing to do is change RegisterProtocol(protocol string, bufferSize int) chan Message to RegisterProtocol(protocol string, chan Message)
of course that means that you will have to create and pass the chan when calling RegisterProtocol.
|
closed due to lack of activity |
Hi, please check the PR and see if the requirement is satisfied