-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Update according to review in issue #1159 #1165
Conversation
RxJava-pull-requests #1080 SUCCESS |
@@ -2580,7 +2580,7 @@ trait Observable[+T] | |||
* <p> | |||
* <img width="640" src="https://raw.github.com/wiki/Netflix/RxJava/images/rx-operators/doOnTerminate.png"> | |||
* <p> | |||
* This differs from `finallyDo` in that this happens BEFORE onCompleted/onError are emitted. | |||
* This differs from `finallyDo` in that this happens **before** `onCompleted/nError` are emitted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nError
=> onError
;-)
@benjchristensen could you wait for this PR ready and merge it before you release |
RxJava-pull-requests #1082 SUCCESS |
@@ -167,7 +167,7 @@ class CompletenessTest extends JUnitSuite { | |||
"zip(Iterable[_ <: Observable[_]], FuncN[_ <: R])" -> "[use `zip` in companion object and `map`]" | |||
) ++ List.iterate("T", 9)(s => s + ", T").map( | |||
// all 9 overloads of startWith: | |||
"startWith(" + _ + ")" -> "[unnecessary because we can just use `::` instead]" | |||
"startWith(" + _ + ")" -> "[unnecessary because we can just use `++` instead]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could differentiate here: instead of startWith(T)
, use +:
, and instead of startWith(more than one T)
, use Observable.items(...) ++ o
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Already updated it.
RxJava-pull-requests #1083 SUCCESS |
@@ -167,7 +167,7 @@ class CompletenessTest extends JUnitSuite { | |||
"zip(Iterable[_ <: Observable[_]], FuncN[_ <: R])" -> "[use `zip` in companion object and `map`]" | |||
) ++ List.iterate("T", 9)(s => s + ", T").map( | |||
// all 9 overloads of startWith: | |||
"startWith(" + _ + ")" -> "[unnecessary because we can just use `::` instead]" | |||
"startWith(" + _ + ")" -> "[unnecessary. Instead of `startWith(T)`, use `+:`, and instead of `startWith(more than one T)`, use `Observable.items(...) ++ o`]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea of this table is to have one row for each item, so you should split this into two. Also, when editing this code, from time to time you should run printMarkdownCorrespondenceTable
, and convert the markdown to html and check if the generated html looks nice.
And you're also welcome to make PRs against https://github.com/RxScala/RxScala.github.io so that your updates are actually read by visitors of the http://rxscala.github.io/comparison.html page ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated it. I'll send a PR to RxScala.github.io once this one is merged.
RxJava-pull-requests #1084 SUCCESS |
Update according to review in issue #1159
/cc @samuelgruetter