-
Notifications
You must be signed in to change notification settings - Fork 641
pubsub: refactor (fixes #98). #107
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
Conversation
lib/pubsub/index.js
Outdated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Added some comments, also applicable to Topic get/create/initialize. |
|
Will get right to it 👍 |
|
Made the updates! |
|
Oops, not to the readme. I'll get to that next. |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
This PR is getting too big to review, can we first work on Subscription only? |
Sure, I can start over. It sounds like we're keeping consistent with the old API, as far as behavior, with some name changes: |
Here's the PubSub API refactor (#98)! The readme updates + tests (regression & unit) will be good places to see it in action. Let me know if you see anything you think can be improved!
Note: this could likely still use a few more tests, and definitely needs a sweep for doc block accuracy. I'll get to these soon.