Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix(pubsub.peers): remove the requirement for a topic #1125

Merged
merged 2 commits into from
Dec 5, 2017
Merged

fix(pubsub.peers): remove the requirement for a topic #1125

merged 2 commits into from
Dec 5, 2017

Conversation

haoliangyu
Copy link
Contributor

fix #1100

@richardschneider
Copy link
Contributor

@diasdavid LGTM. The failed travis CI has nothing to do with this PR

/home/travis/build/ipfs/js-ipfs/src/http/api/routes/webui.js
   1:1   error  Use the global form of 'use strict'  strict
  21:99  error  Extra semicolon                      semi

@@ -77,9 +77,18 @@ module.exports = function pubsub (self) {
return setImmediate(() => callback(new Error(OFFLINE_ERROR)))
}

const peers = Array.from(self._pubsub.peers.values())
.filter((peer) => peer.topics.has(topic))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change required here could just be:

.filter((peer) => topic ? peer.topics.has(topic)) : true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haoliangyu mind updating to the above solution?

@haoliangyu
Copy link
Contributor Author

@diasdavid The change has been made.

const peers = Array.from(self._pubsub.peers.values())
.filter((peer) => peer.topics.has(topic))
.filter((peer) => topic ? peer.topics.has(topic) : true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bother doing the filter if you're getting all peers anyway?

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.

pubsub/peers topic is optional
4 participants