refactor: ping #505
refactor: ping #505
Conversation
src/ping/handler.js
Outdated
| * Subscribe ping protocol handler. | ||
| * @param {Libp2p} node | ||
| */ | ||
| function mount (node) { |
There was a problem hiding this comment.
@jacobheun what do you think about these namings?
I thought that we could make this more consistent with the transports and have a createListener function or similar. But, I decided to gather your feedback first.
There was a problem hiding this comment.
I think mount/unmount is okay. We're not so much creating listeners as we are attaching an application protocol to libp2p. Different services/protocols may have different needs so keeping it generic might be good. If we ever create an interface for this type of service we could reevaluate this then.
vasco-santos
force-pushed
the
refactor/ping
branch
2 times, most recently
from
16d1cc0 to
7f7d7ea
Compare
vasco-santos
force-pushed
the
refactor/ping
branch
from
7f7d7ea to
25463e1
Compare
vasco-santos
changed the base branch from
feat/token-dialer
to
refactor/async-await
vasco-santos
force-pushed
the
refactor/ping
branch
from
92c0bcd to
c456834
Compare
vasco-santos
force-pushed
the
refactor/ping
branch
from
c456834 to
e547009
Compare
src/index.js
Outdated
| * @returns {Promise<number>} | ||
| */ | ||
| async ping (peer) { | ||
| const peerInfo = await getPeerInfoRemote(peer, this) |
There was a problem hiding this comment.
I think we should just use getPeerInfo for this, as ping really shouldn't support peers we don't know about locally. getPeerInfoRemote will search the network if it can't find it locally.
* refactor: ping * chore: ping is now a function * chore: address review
* refactor: ping * chore: ping is now a function * chore: address review
This PR refactors ping to use
async-itinstead ofpull-streamsNeeds: