-
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
2.x: Expand the Getting Started #5926
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #5926 +/- ##
============================================
+ Coverage 98.24% 98.25% +0.01%
+ Complexity 6021 6017 -4
============================================
Files 656 656
Lines 44040 44040
Branches 6102 6102
============================================
+ Hits 43265 43272 +7
+ Misses 235 226 -9
- Partials 540 542 +2
Continue to review full report at Codecov.
|
Haven't encountered any rendering problems. |
README.md
Outdated
The dataflows in RxJava consist of a source, zero or more intermediate steps followed by a data consumer or combinator step (where the step is responsible to consume the dataflow by some means): | ||
|
||
```java | ||
|
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.
are those new lines necessary?
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.
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.
In this case though, there was a line there which got accidentally deleted.
The first step is to include RxJava 2 into your project, for example, as a Gradle compile dependency: | ||
|
||
```groovy | ||
compile "io.reactivex.rxjava2:rxjava:2.x.y" | ||
``` | ||
|
||
(Please replace `x` and `y` with the latest version numbers: [![Maven Central](https://maven-badges.herokuapp.com/maven-central/io.reactivex.rxjava2/rxjava/badge.svg)](https://maven-badges.herokuapp.com/maven-central/io.reactivex.rxjava2/rxjava) |
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.
Badge image is not displaying, I think [
before !
is not needed
https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet#images
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.
Ah, you want to make it clickable, then something wrong with syntax or my browser (Firefox 59.0.1)
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.
It is the same markdown as in the beginning of the readme file where the top badges are located. Sometimes the maven badge simply fails to load. I'm assuming this is due to throttling at the host service where maven-badges lives.
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.
Great rewrite!
Sorry can't finish it right now (don't want to block merge though), posting partial review
- [`io.reactivex.Completable`](http://reactivex.io/RxJava/2.x/javadoc/io/reactivex/Completable.html): a flow without items but only a completion or error signal, | ||
- [`io.reactivex.Maybe`](http://reactivex.io/RxJava/2.x/javadoc/io/reactivex/Maybe.html): a flow with no items, exactly one item or an error. | ||
|
||
### Some terminology |
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.
nit: "Some" → "Common"?
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.
These are not all the terminology but those which appear later in the text and I felt I have to define them upfront.
|
||
#### Upstream, downstream | ||
|
||
The dataflows in RxJava consist of a source, zero or more intermediate steps followed by a data consumer or combinator step (where the step is responsible to consume the dataflow by some means): |
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.
"combinator" → "operator"?
I see few other usages of "combinator" https://github.com/ReactiveX/RxJava/search?q=combinator&type=Code&utf8=%E2%9C%93 but idk, doesn't look like common term in RxJava community
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 dataflows in RxJava consist of a source, zero or more intermediate steps followed by a data consumer or combinator step (where the step is responsible to consume the dataflow by some means): | ||
|
||
```java | ||
source.operator1().operator2().operator3().subscribe(consumer); |
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.
Here we see "operator" instead of "combinator", so something should be changed for consistency I guess
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.
Here operators are the steps, the combinator refers to flatMap
.
.subscribe(consumer) | ||
``` | ||
|
||
#### Objects in motion |
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.
Never heard "Objects in motion" used to describe RxJava streams
https://github.com/reactive-streams/reactive-streams-jvm#glossary
Can't come up with good alternative atm, something along with "Dataflow" could work
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.
I had to encompass all the terms below without repeating them in the title. This felt like a nice encompassing term.
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.
yeah I understand, up2u of course
But just in case, headers in markdown can be clicked and used in urls, so changes in future will break existing urls
That's why I decided to comment it even though I don't have a good replacement option atm
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.
I'll try to remember and add hidden anchors <a name="objects-in-motion"></a>
if the time comes.
flow.subscribe(System.out::println) | ||
```` | ||
|
||
This is when the **subscription side-effects** are triggered (see `doOnSubscribe`). Some sources block or start emitting items right away in this state. |
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.
Maybe reword to something like:
"Typically in this state source starts synchronous or asynchronous work to prepare data to be 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.
I think referring to "block" and "emitting" is more appropriate in a getting started guide.
|
||
This is when the **subscription side-effects** are triggered (see `doOnSubscribe`). Some sources block or start emitting items right away in this state. | ||
|
||
#### Runtime |
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.
I know it's subjective, but Runtime
reads as noun, an object here rather than set of states in time.
Maybe change to "Run-time"?
"“Runtime”, “run time”, and “run-time”": https://english.stackexchange.com/a/197688
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.
I think we need a native English speaker for this.
This PR expands the Readme.md with further examples and details, plus adds headers so that (former and new) sections can be easily linked in answers on issue lists or StackOverflow.
(Note that it was written offline without proper markdown rendering so I may update the PR a couple of times to get the render mistakes fixed).