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

fix: make pubsub.unsubscribe async and alter pubsub.subscribe signature #1348

Merged
merged 7 commits into from
May 14, 2018

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented May 9, 2018

@ghost ghost assigned alanshaw May 9, 2018
@ghost ghost added the status/in-progress In progress label May 9, 2018
@victorb
Copy link
Member

victorb commented May 9, 2018

Just a headup, the commit should contain "Breaking change" somewhere

Edit: somewhere being the footer of commit message

BREAKING CHANGE: pubsub.unsubscribe is now async and argument order for pubsub.subscribe has changed

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
@alanshaw alanshaw changed the title [WIP] fix: make pubsub.unsubscribe async and alter pubsub.subscribe signature fix: make pubsub.unsubscribe async and alter pubsub.subscribe signature May 9, 2018
@alanshaw alanshaw requested a review from daviddias May 9, 2018 21:39
return Promise.resolve()
}

process.nextTick(() => callback())
Copy link
Member

Choose a reason for hiding this comment

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

not all bundlers know how to handle process.nextTick well. We always use async/setImmediate for that matter.

@ghost ghost assigned daviddias May 12, 2018
alanshaw and others added 4 commits May 12, 2018 17:58
BREAKING CHANGE: pubsub.unsubscribe is now async and argument order for pubsub.subscribe has changed

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
@alanshaw
Copy link
Member Author

@diasdavid tests not passing here because ipfsd-ctl depends on ipfs-api@^20. I'll submit a PR there now.

@alanshaw
Copy link
Member Author

@diasdavid pubsub tests now pass for me when I npm link ipfsd-ctl on this branch ipfs/js-ipfsd-ctl#245

@daviddias
Copy link
Member

@alanshaw ipfsd-ctl 0.34 is out :)

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
@alanshaw
Copy link
Member Author

🎉 only unrelated DNS test failed...

@daviddias daviddias merged commit 7362652 into master May 14, 2018
@ghost ghost removed the status/in-progress In progress label May 14, 2018
@daviddias daviddias deleted the fix/pubsub-unsub-async branch May 14, 2018 12:43
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.

3 participants