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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use n-args for all static combineLatest signatures #6092

Merged

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented Mar 6, 2021

Description:

This PR uses the n-args mechanism for all of the static combineLatest's overload signatures.

As mentioned in a couple of other PRs: I realize that these sigs are deprecated, but we already have deprecated sigs in operators that use n-args, so, IMO, things are less confusing if n-args is useds wherever possible.

Further to that:

  • The sheer number of overloads removed makes this source much easier to read and reason with.
  • It will make it easier to manage the deprecation comments - after the breaking changes are done and v7 is ready, getting the deprecations sorted/fixed and relating them to the docs is something that needs to be done, IMO, as the deprecations confuse many devs.
  • There has been some discussion about undeprecating some result selectors and - IMO - this is one API where the result selector is useful.
  • Having the types behave differently for depreated and non-deprecated APIs is likely to confuse people.
  • The Scheduler-related APIs that are deprecated are unlikely to be widely used.
  • The ObservableInput-based type parameters are pretty unlikely to be specified explicitly anyway.
  • If you're still not convinced, let me know and I'll come up with some more reasons, 'cause I really wanna get rid of those overload sigs. 馃檪

Related issue (if exists): Nope

@benlesh
Copy link
Member

benlesh commented Mar 8, 2021

Approved... but needs rebased after the other PR was merged.

@cartant cartant force-pushed the cartant/static-combine-latest-n-args branch from 8c84722 to faa6c8d Compare March 9, 2021 07:58
@cartant cartant merged commit 5e95503 into ReactiveX:master Mar 9, 2021
@cartant cartant deleted the cartant/static-combine-latest-n-args branch March 19, 2021 07:21
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.

None yet

2 participants