Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PUBSUB: unsubscribe without handler reference #321

Merged
merged 2 commits into from
Feb 21, 2019

Conversation

Isan-Rivkin
Copy link
Contributor

@Isan-Rivkin Isan-Rivkin commented Feb 6, 2019

Hi :-)
following up on an issue in interface-js-ipfs-core I'm creating this PR.
Also created PR in js-ipfs-http-client accordingly.
This would help in cases like mine where my implementation is not able to support references to a handler function.
I will add the final url in the README to the method documentation once I merge the final PR to interface-js-ipfs-core since its pointing nowhere now.

Edit:

Opened a PR on interface-js-ipfs-core

@jacobheun
Copy link
Contributor

Per ipfs-inactive/interface-js-ipfs-core#437 (comment), I think we should just overload unsubscribe.

cc @vasco-santos

@jacobheun
Copy link
Contributor

Also, when you update, can you fix linting npm run lint?

@Isan-Rivkin
Copy link
Contributor Author

@jacobheun thanks, I commented in interface-js-ipfs.
Will do the linting, thanks for the hint I forgot.

@vasco-santos
Copy link
Member

I think we should just overload unsubscribe.

Subscribe this point

@Isan-Rivkin Isan-Rivkin force-pushed the master branch 4 times, most recently from c6f56c2 to 4016320 Compare February 20, 2019 19:52
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @Isan-Rivkin!

@jacobheun jacobheun merged commit 6e76aad into libp2p:master Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants