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

pubsub/peers topic is optional #1100

Closed
richardschneider opened this issue Nov 22, 2017 · 3 comments · Fixed by #1125
Closed

pubsub/peers topic is optional #1100

richardschneider opened this issue Nov 22, 2017 · 3 comments · Fixed by #1125
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)

Comments

@richardschneider
Copy link
Contributor

richardschneider commented Nov 22, 2017

According to the docs the topic argument is optional in pubsub/peers.

js-ipfs makes it mandatory.

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue labels Nov 24, 2017
@daviddias
Copy link
Member

Good catch. Open for contribs!

@haoliangyu
Copy link
Contributor

haoliangyu commented Dec 2, 2017

I can work on this issue.

Another place that (maybe) needs change is here where topic is used to filter the peers to return. If the topic is optional, the filtering could be skipped based on the value of topic.

Since the topic is optional, do you want to keep the pubshub.peers(topic, callback) pattern even the topic is nullable, or allow pubshub.peers(callback) when topic is undefined?

Please advice.

@richardschneider
Copy link
Contributor Author

@haoliangyu Good analysis. I like having two method signatures

  • pubsub.peers (callback)
  • pubsub.peers (topic, callback)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants