Skip to content

refactor(merge): support N-args with concurrency and scheduler#5877

Merged
benlesh merged 2 commits intoReactiveX:masterfrom
cartant:static-merge-n-args
Nov 17, 2020
Merged

refactor(merge): support N-args with concurrency and scheduler#5877
benlesh merged 2 commits intoReactiveX:masterfrom
cartant:static-merge-n-args

Conversation

@cartant
Copy link
Copy Markdown
Collaborator

@cartant cartant commented Nov 6, 2020

Description:

#5844 was closed in favour of #5859, but that PR refactored only the operators - not the static merge.

This PR refactors the static merge to use ObservableInputTuple, etc.

FWIW, I'm aware that there is an argsOrArgArray call in the implementation, but IMO we should not be introducing signatures that allow arrays of sources to be passed to merge. AFAICT, there were no such signatures prior to the refactor and there are no such signatures in version 6. Furthermore, there are behavioural differences between concat and merge because of this. Basically, I've spent a chunk of time compiling a table of the signature and implementation differences (for v6 and v7) relating to arrays of sources and varargs. I'd like to keep this PR as simple as possible and address said differences in other PRs/issues. AFAICT, there are numerous inconsistencies.

Related issue (if exists): None

@cartant cartant force-pushed the static-merge-n-args branch from b7aaa0f to 52d5130 Compare November 7, 2020 04:51
@cartant cartant force-pushed the static-merge-n-args branch from ccb6893 to 5c5da02 Compare November 8, 2020 04:55
@cartant cartant changed the title WIP: refactor(merge): support N-args with concurrency and scheduler refactor(merge): support N-args with concurrency and scheduler Nov 8, 2020
@cartant cartant marked this pull request as ready for review November 8, 2020 04:57
@cartant cartant requested review from benlesh and kolodny November 8, 2020 04:57
@benlesh benlesh merged commit 646d022 into ReactiveX:master Nov 17, 2020
@cartant cartant deleted the static-merge-n-args branch March 19, 2021 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants