Switches to positive wording for all rules besides 2.3, #200
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
| @@ -134,11 +134,11 @@ public interface Subscription { | |||
|
|
|||
| | ID | Rule | | |||
| | ------------------------- | ------------------------------------------------------------------------------------------------------ | | |||
| | <a name="3.1">1</a> | `Subscription.request` or `Subscription.cancel` MUST not be called outside of its `Subscriber` context. A `Subscription` represents the unique relationship between a `Subscriber` and a `Publisher` [see [2.12](#2.12)] | | |||
| | <a name="3.1">1</a> | `Subscription.request` and `Subscription.cancel` MUST only be called inside of its `Subscriber` context. A `Subscription` represents the unique relationship between a `Subscriber` and a `Publisher` [see [2.12](#2.12)] | | |||
There was a problem hiding this comment.
Switched from or to and here, since that was the intent. Is it clearer?
viktorklang
force-pushed
the
wip-197-positive-wording-√
branch
from
182ab29 to
208e778
Compare
| | <a name="3.4">4</a> | `Subscription.request` SHOULD NOT synchronously perform heavy computations that would impact its callers responsivity | | ||
| | <a name="3.5">5</a> | `Subscription.cancel` MUST NOT synchronously perform heavy computations, MUST be idempotent and MUST be thread-safe | | ||
| | <a name="3.3">3</a> | `Subscription.request` MUST only allow bounded recursion such as `Subscriber.onNext` -> `Subscription.request` -> `Subscriber.onNext` | | ||
| | <a name="3.4">4</a> | `Subscription.request` SHOULD respect the responsivity of its caller by returning in a timely manner | |
There was a problem hiding this comment.
Reason for the change here is to put the focus on the responsivity not on how "heavy" a computation may or may not be.
There was a problem hiding this comment.
Should we keep “performing heavy computations” as an undesirable example in parentheses?
There was a problem hiding this comment.
Adding footnote
|
Very good initiative, avoiding negation makes the intent clearer in all those cases. |
|
LGTM |
viktorklang
force-pushed
the
wip-197-positive-wording-√
branch
2 times, most recently
from
18cde01 to
0632c9f
Compare
simply because I could not find appropriate wording. NOTE: No semantical changes have been made AFAICT. Harmonises the use of "call" and "invoke" to use "call" in all places where "invoke" was used. Adds a couple of footnotes to `Subscription` rules to keep the rules free from examples.
viktorklang
force-pushed
the
wip-197-positive-wording-√
branch
from
0632c9f to
db71dfe
Compare
| [<a name="footnote-3-1">1</a>] : As it is not feasibly reachable with current or forseen hardware within a reasonable amount of time (1 element per nanosecond would take 292 years) to fulfill a demand of 2^63-1. | ||
| [<a name="footnote-3-1">1</a>] : An example for undesirable synchronous, open recursion would be `Subscriber.onNext` -> `Subscription.request` -> `Subscriber.onNext` -> …, as it very quickly would result in blowing the calling Thread´s stack. | ||
|
|
||
| [<a name="footnote-3-2">2</a>] : Avoid heavy computations and other things that would stall the caller´s thread of execution |
There was a problem hiding this comment.
@rkuhn This good?
Closed
|
@reactive-streams/contributors Merging! |
viktorklang
added a commit
that referenced
this pull request
Jan 22, 2015
Switches to positive wording for all rules besides 2.3,
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.
simply because I could not find appropriate wording.
NOTE: No intentional semantical changes have been made. If you find issues, please, please, please let me know
Harmonises the use of "call" and "invoke" to use "call" in all places
where "invoke" was used.
@reactive-streams/contributors Could you please sanity check the changes proposed here? Thanks!
Please vote/comment before the 21st of January (as always, if you need more time let us know)