Skip to content
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: Add marbles for Single.concat operator #6137

Merged
merged 3 commits into from
Aug 5, 2018

Conversation

UMFsimke
Copy link
Contributor

@UMFsimke UMFsimke commented Aug 4, 2018

Here are marbles for concat(Iterable), concat(Publisher), concat(Publisher, prefetch), concat(Observable), concat(source1, source2), concat(source1, source2, source3), concat(source1, source2, source3, source4) and concatArray operators that should close them at #5788 .

Marble for concat(Iterable)
concatIterable

Marble for concat(Publisher)
concatPublisher

Marble for concat(Publisher, prefetch)
concatPublisherN

Marble for concat(Observable)
concatObservable

Marble for concat(source1, source2)
concat2

Marble for concat(source1, source2, source3)
concat3

Marble for concat(source1, source2, source3, source4)
concat4

Marble for concatArray
concatArray

Please pay attention to file names in javadoc as I was not sure how to name concat(Iterable) so I've added at the end i, for concat(Observable) I've added o. For concat(source1, source2, source3) and concat(source1, source2, source3, source4) I've used n and m as they are numerical values.

Also, is there an error in concat(Publisher) and concat(Publisher, prefetch) with request() commands? I did used as a reference Completable.concat(Publisher) marble but I do not understand why do we have request(1) after first event as we already requested prefetching of multiple ones. Is it because others are still not produced?

Thanks

@UMFsimke UMFsimke changed the title Add marbles for Single.concat operator 2.x: Add marbles for Single.concat operator Aug 4, 2018
@codecov
Copy link

codecov bot commented Aug 4, 2018

Codecov Report

Merging #6137 into 2.x will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6137      +/-   ##
============================================
+ Coverage     98.25%   98.26%   +0.01%     
+ Complexity     6197     6194       -3     
============================================
  Files           667      667              
  Lines         44853    44853              
  Branches       6213     6213              
============================================
+ Hits          44069    44074       +5     
+ Misses          237      236       -1     
+ Partials        547      543       -4
Impacted Files Coverage Δ Complexity Δ
src/main/java/io/reactivex/Single.java 100% <ø> (ø) 146 <0> (ø) ⬇️
...reactivex/internal/operators/single/SingleAmb.java 96.36% <0%> (-3.64%) 9% <0%> (-1%)
...a/io/reactivex/internal/util/QueueDrainHelper.java 97.22% <0%> (-2.78%) 56% <0%> (-2%)
.../operators/mixed/FlowableSwitchMapCompletable.java 98.94% <0%> (-1.06%) 2% <0%> (ø)
...perators/observable/ObservableMergeWithSingle.java 99.06% <0%> (-0.94%) 2% <0%> (ø)
.../operators/maybe/MaybeFlatMapIterableFlowable.java 97.54% <0%> (-0.82%) 2% <0%> (ø)
...ternal/operators/observable/ObservableFlatMap.java 87.22% <0%> (-0.64%) 3% <0%> (ø)
...x/internal/operators/flowable/FlowableGroupBy.java 95.51% <0%> (-0.57%) 3% <0%> (ø)
...ex/internal/operators/flowable/FlowableCreate.java 97.41% <0%> (-0.33%) 6% <0%> (ø)
...x/internal/operators/flowable/FlowableFlatMap.java 89.47% <0%> (-0.27%) 4% <0%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c146374...4f5004b. Read the comment docs.

@akarnokd
Copy link
Member

akarnokd commented Aug 5, 2018

  • Please color the marbles in the sequences differently to each other.
  • Since some inputs and outputs are Observables and Flowables, their terminal event idicator is a pass-throug arrow: ----|-----> and ----X----->. Please update these diagrams accordingly.
  • Use Single.concat.o3.png and Single.concat.o4.png.
  • The X in the middle of the left side of concat(Publisher, prefetch) is misaligned.
  • Multiple requests are there to reinforce those are Flowables and thus more requests may come even if there was some initial request before. Keep them in the diagram.

@UMFsimke
Copy link
Contributor Author

UMFsimke commented Aug 5, 2018

Thanks for explanation!

Marbles are now updated per comments

@akarnokd
Copy link
Member

akarnokd commented Aug 5, 2018

The concat(Publisher) and concat(Publisher, int) diagram's input side and inside the box are still ---->|

@UMFsimke
Copy link
Contributor Author

UMFsimke commented Aug 5, 2018

Based on a comment I thought that only Observables and Flowables should be updated. Those images are now updated with ----|---->

@akarnokd akarnokd merged commit bb93911 into ReactiveX:2.x Aug 5, 2018
@UMFsimke UMFsimke deleted the SingleMarbles85a branch August 5, 2018 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants