document rationale for void return of subscribe() #51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -90,7 +92,9 @@ In addition there is one method each on Producer and Consumer to obtain a refere | |||
|
|
|||
| The Reactive Streams SPI prescribes that all processing of elements (onNext) or termination signals (onError, onComplete) happens outside of the execution stack of the Publisher. This is achieved by scheduling the processing to run asynchronously, possibly on a different thread. The Subscriber should make sure to minimize the amount of processing steps used to initiate this process, meaning that all its SPI-mandated methods shall return as quickly as possible. | |||
|
|
|||
| In contrast to communicating back-pressure by blocking the publisher, a non-blocking solution needs to communicate demand through a dedicated control channel. This channel is provided by the Subscription: the subscriber controls the maximum amount of future elements it is willing receive by sending explicit demand tokens (by calling requestMore(int)). | |||
| In contrast to communicating back-pressure by blocking the publisher, a non-blocking solution needs to communicate demand through a dedicated control channel. This channel is provided by the Subscription: the subscriber controls the maximum amount of future elements it is willing receive by sending explicit demand tokens (by calling requestMore(int)). | |||
There was a problem hiding this comment.
this line is signaled as changed because my editor removed the before its end; we should settle on Unix line encoding and fix the text files at some point
|
Is this change good to go? |
|
It's conflicting with the README merge that just went in. |
|
Moved the sentences to their new locations and pushed a rebase: there were no LGTMs that could have been invalidated. |
|
LGTM |
1 similar comment
|
LGTM |
|
@reactive-streams/contributors can this go in? |
|
👍 |
1 similar comment
|
+1 |
|
LGTM |
| @@ -157,6 +156,7 @@ All of these variants are "asynchronous streams". They all have their place and | |||
|
|
|||
| The Reactive Streams contract allows implementations the flexibility to manage resources and scheduling and mix asynchronous and synchronous processing within the bounds of a non-blocking, asynchronous, push-based stream. | |||
|
|
|||
| In order to allow fully asynchronous implementations of all participating SPI elements—`Publisher`/`Subscription`/`Subscriber`—all methods defined by these interfaces return `void`. | |||
There was a problem hiding this comment.
The README currently only uses the term API but this uses SPI. Is that on purpose? Does it confuse things?
|
I'm good with these changes. Only possible thing to change is SPI/API term. |
|
I'll finalize on 0.4.0.M1 PR #61 @benjchristensen |
smaldini
added a commit
that referenced
this pull request
Jun 3, 2014
document rationale for void return of subscribe()
smaldini
pushed a commit
that referenced
this pull request
Jun 3, 2014
|
Thanks, @smaldini ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #43