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

Feature: MergeMany Support for OnComplete (List Version) #742

Merged

Conversation

dwcullop
Copy link
Member

@dwcullop dwcullop commented Oct 23, 2023

Description

Updates the List version of MergeMany to have the same behavior added to the cache version (#736): The resulting sequence will fire OnComplete when the source sequence and all child sequences complete. It is the same behavior as Observable.SelectMany.

All of the changes are identical to the changes made to the cache version. The SubscriptionCounter class could be abstracted out and shared between them, but that doesn't seem to be consistent with how things work in this repo, so I didn't do it that way.

This change is needed for the upcoming List version of MergeManyChangeSets (still in progress) but this part is done, with unit tests, and it makes sense to have it as a separate PR.

@dwcullop dwcullop changed the title Feature: MergeMany Update for List Feature: MergeMany Support for OnComplete (List Version) Oct 23, 2023
@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #742 (fe7dede) into main (b01fe12) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #742      +/-   ##
==========================================
+ Coverage   64.29%   64.36%   +0.06%     
==========================================
  Files         222      222              
  Lines       11224    11246      +22     
  Branches     2271     2273       +2     
==========================================
+ Hits         7217     7239      +22     
  Misses       3062     3062              
  Partials      945      945              
Files Coverage Δ
src/DynamicData/List/Internal/MergeMany.cs 93.75% <100.00%> (+13.75%) ⬆️

@RolandPheasant RolandPheasant merged commit b30102f into reactivemarbles:main Oct 26, 2023
3 checks passed
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2023
@dwcullop dwcullop deleted the feature/list-mergemany-update branch November 23, 2023 01:00
@dwcullop dwcullop self-assigned this Dec 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants