Uh oh!
There was an error while loading. Please reload this page.
pubsub refactor - #245
Conversation
905e1c0 to
b31f443Comparestephenplusplus
commented
Oct 1, 2014
b31f443 follows up from recent discussions in #98, with a change:
|
ryanseys
commented
Oct 1, 2014
The Also, is the only way to stop listening to pass in the "listener" (callback)? My puny brain doesn't quite understand Events fully yet. To support multiple listeners this makes the most sense, just curious if there was a better way? |
stephenplusplus
commented
Oct 1, 2014
I think it's too specific of a parameter to applied generally. returnImmediately false is wanted for auto pulling and returnImmediately true may or may not be wanted for individual pulls.
An event can, and often will, have multiple handlers assigned. When one is no longer needed, it must be removed individually by passing the name of the event and the listener assigned. As long as there is still one active listener to the message event, pulling will continue. Once no more exist, pulling stops. A user can run The nice thing about this is we are following the standard events spec, and not requiring any extra method calls. |
stephenplusplus
commented
Oct 1, 2014
Yes.
Either way. It's only of use to the one function, so I scoped it to it. |
ryanseys
commented
Oct 1, 2014
We can get the listeners and .length() it from EventEmitter.listeners() |
stephenplusplus
commented
Oct 1, 2014
The listeners could be of varying types, |
ryanseys
commented
Oct 1, 2014
So if we only specify subscription.on('error', listener) it actually won't pull? |
stephenplusplus
commented
Oct 2, 2014
Yes, this design can be changed, but I think it would only make sense to listen for messages. We could also run into a problem where a message is pulled & emitted after an error handler was assigned but before a message handler was assigned. |
ryanseys
commented
Oct 2, 2014
Makes sense. |
b31f443 to
68848aaComparestephenplusplus
commented
Oct 2, 2014
Updated to address:
|
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
ryanseys
commented
Oct 2, 2014
Overall looks good. |
ryanseys
commented
Oct 5, 2014
Are we awaiting additional reviews before shipping? Can I help? |
71572bb to
3acb446Comparestephenplusplus
commented
Oct 6, 2014
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/00d271af-f95c-45b1-89db-4a1ec8c1dbff/targets - [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.) Source-Link: googleapis/synthtool@7332178
🤖 I have created a release \*beep\* \*boop\* --- ## [2.1.0](https://www.github.com/googleapis/nodejs-projectify/compare/v2.0.1...v2.1.0) (2021-06-10) ### Features * add `gcf-owl-bot[bot]` to `ignoreAuthors` ([#245](https://www.github.com/googleapis/nodejs-projectify/issues/245)) ([30f0499](https://www.github.com/googleapis/nodejs-projectify/commit/30f0499ade5f140774c3aa672b44fd3538e72309)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Co-authored-by: Benjamin E. Coe <bencoe@google.com> Co-authored-by: Justin Beckwith <justin.beckwith@gmail.com>
resolves#98
A good portion of this change is to the docs/ directory to support the addition of a module. In this case, PubSub will only be documented on the site once a release contains this pull. To support that, we have to generate the sidebar links dynamically based on the version of the docs the user is browsing.
Most of the changes to do that will likely come with https://github.com/ryanseys/gcloud-node/pull/1, which had the same demands (we are adding a new
Transactionsub-module in that case).So, feel free to zoom by those changes for now. Since this PR drastically changed just about everything, a diff won't be all that friendly of a way to view the changes. Feel free to use https://github.com/stephenplusplus/gcloud-node/tree/spp--pubsub-refactor/lib/pubsub to get a better sense of how everything works, as well as the test directory: https://github.com/stephenplusplus/gcloud-node/tree/spp--pubsub-refactor/test/pubsub