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

Adds optional useReplaceForUpdates to SortedObservableCollectionAdaptor #726

Conversation

ccopsey
Copy link
Contributor

@ccopsey ccopsey commented Sep 21, 2023

In a similar vein to how useReplaceForUpdates in ObservableCollectionAdaptor toggles between Replace and Remove/Add update types, this pull request proposes adding the same capability to SortedObservableCollectionAdaptor.

Default to true for backward compatibility, although if a new major version is in the offing, maybe this could be false to match ObservableCollectionAdaptor.

The test has been updated to match the equivalent test in ObservableCollectionBindCacheFixture.

@ccopsey
Copy link
Contributor Author

ccopsey commented Sep 22, 2023

The failing test has nothing to do with me. I will wait to pull a fix for that.

@RolandPheasant
Copy link
Collaborator

The failing test has nothing to do with me. I will wait to pull a fix for that.

Years ago I made a test private as it was flaky, occasionally failed and was a false negative. For some strange reason the test runs after a minor version test library update, so I have removed the [Fact] annotation altogether to ignore the test.

Just to warn you when you rebase, there are a load of changes as I merged a PR which applies a not null restraint to cache and list items. This may lead to conflicts.

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #726 (ae8cd2e) into main (4e9a209) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##             main     #726   +/-   ##
=======================================
  Coverage   71.74%   71.74%           
=======================================
  Files         216      216           
  Lines       10893    10894    +1     
=======================================
+ Hits         7815     7816    +1     
  Misses       3078     3078           
Files Changed Coverage Δ
src/DynamicData/Cache/ObservableCacheEx.cs 50.65% <0.00%> (ø)
...cData/Binding/SortedObservableCollectionAdaptor.cs 83.78% <100.00%> (+0.45%) ⬆️

Copy link
Collaborator

@RolandPheasant RolandPheasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@RolandPheasant RolandPheasant merged commit b3180bc into reactivemarbles:main Sep 22, 2023
3 checks passed
@ccopsey ccopsey deleted the feature/sortedobservablecollectionadapter-usereplaceforupdates branch September 22, 2023 09:31
@github-actions
Copy link

github-actions bot commented Oct 7, 2023

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 Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants