-
-
Notifications
You must be signed in to change notification settings - Fork 182
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: MergeManyChangeSets for Cache ChangeSets with List ChangeSets #790
Feature: MergeManyChangeSets for Cache ChangeSets with List ChangeSets #790
Conversation
…ub.com/dwcullop/DynamicData into feature/cache-list-mergemanychangesets
- Code Cleanup - Move Synchronization call - Remove extra Cache / List that isn't needed - Add EqualityComparer to List-to-List version
Code wise, it looks great and only one minor request which is to seal classes when possible. Aside from that I ran the stress tests and managed to produce some intermittent errors. and on a subsequent run I think the stress tests are a brilliant idea, but I wonder whether that test performance wizard @JakenVeina has any suggestions how those tests can be made to run fast, |
…ub.com/dwcullop/DynamicData into feature/cache-list-mergemanychangesets
Would be happy to, if the branch would build. ;) |
SourceErrorsImmediately_SubscriptionReceivesError is now re-enabled I ran on a loop test and passed every time
The problem now is that the Stress tests take an infinite period of time to run. |
@JakenVeina The branch should build now, if you have time |
…ub.com/dwcullop/DynamicData into feature/cache-list-mergemanychangesets
Agree about sealing the classes, but I can't find any that weren't already sealed (or static).
Been fighting these for a few days now, but I hope I've got them under control now.
I'm open to any suggestions. |
…n moved to reactivemarbles#796 This reverts commit 315f013.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not followed the entire conversation on Slack, owning to being away for a couple days. Instead for now I'll approve and leave to the discretion of @dwcullop whether it is ready to be merged.
…ub.com/dwcullop/DynamicData into feature/cache-list-mergemanychangesets
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. |
Description
Implements the
MergeManyChangeSets
operator for Cache ChangeSets source and List ChangeSet children. This PR also adds anIEqualityComparer
to the List-to-List version that was omitted before.Other Changes
MergeManyCacheChangeSetsSourceCompareFixture
toMergeManyChangeSetsCacheSourceCompareFixture
for consistencyCompleted
toIsCompleted
property on Cache Aggregator for consistency