Move disposable helpers into core primitives and update Extensions#14
Merged
Conversation
Update the migrated SyncTimerObservable test to assert the disposed middle subscription stops receiving ticks after disposal instead of assuming only one tick occurred before disposal. This preserves the existing TimeSpan.Zero first-tick SyncTimer contract from ReactiveUI.Extensions while making the test deterministic across Windows, Linux, and macOS virtual scheduler behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
==========================================
+ Coverage 89.09% 91.50% +2.40%
==========================================
Files 306 398 +92
Lines 12192 15481 +3289
Branches 1840 2231 +391
==========================================
+ Hits 10863 14166 +3303
+ Misses 1010 993 -17
- Partials 319 322 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add targeted ReactiveUI.Primitives.Extensions tests for the Codecov patch recommendations on PR #14. Covers ConcurrencyLimiter disposal/error paths, BooleanReduce materialization branches, Sequencer periodic disposal guards, ObserverExtensions read-only list handling, DropIfBusy post-terminal and error forwarding paths, RetryForever post-dispose completion, SubscribeAsync deferred terminal branches, Continuation double-dispose, and default WaitForError overloads. Verified locally with: dotnet build .\src\ReactiveUI.Primitives.slnx -c Release --no-restore; direct TUnit net8 coverage run 2050/2050 with ReactiveUI.Primitives.Extensions at 100% line and branch coverage; direct TUnit net9 2050/2050; direct TUnit net10 2050/2050.
Remove machine-specific AdditionalFiles entries that referenced a local BannedSymbols.txt path from multiple project files and delete per-target SupportedOSPlatformVersion PropertyGroups from ReactiveUI.Primitives.csproj. These changes remove embedded developer-specific paths and redundant platform version blocks so platform/support settings can be managed centrally. Also note the ReactiveUI.Primitives.Extensions.csproj header now contains a UTF-8 BOM character.
Replace the synthetic disposedTwice assignment with a real no-exception assertion so SonarCloud's IDE0059 rule does not fail the PR analysis. Verified with: dotnet build .\src\tests\ReactiveUI.Primitives.Tests\ReactiveUI.Primitives.Tests.csproj -c Release --no-restore.
Add a new Skills.md containing agent-oriented guidance for consuming ReactiveUI.Primitives from NuGet and migration/bridge instructions. Update README.md to include an "Agent Skills" section and update the table of contents and TFM listings to reference the new guidance. Package Skills.md into the ReactiveUI.Primitives NuGet by adding it to ReactiveUI.Primitives.csproj. Expand supported .NET Framework TFMs to include net48 in src/Directory.Build.props and update optional package TFM lists in the README accordingly.
Resolves conflicts from main's sink-publicization + test-split work: - Directory.Build.props: adopt the NetTargetFrameworks rename (keep net48 in the framework target comment). - Disposable.Create: keep AddExtensions' ActionDisposable; drop main's now-unused AnonymousDisposable. - EmptyDisposable: keep AddExtensions' Instance-singleton version. - Main test csproj: keep both the Async and new Extensions project references. - AsyncTestHelpers moved to tests/Shared and linked into both the main and Async test projects (the Extensions tests use WaitForConditionAsync, which main moved into the Async.Tests project).
- ScanWithInitialTests / ReactiveExtensionsTests.Catch (CA2201): throw InvalidOperationException instead of the reserved base Exception type. - SequencerPeriodicMixins.PeriodicSubscription + Tick are now internal (the Extensions assembly already grants InternalsVisibleTo to the test assembly), so the coverage test invokes Tick() directly instead of via reflection, dropping the S3011 and IL2075 suppressions.
SignalFromTaskHandlesTokenCancellation and SignalFromTask_T_HandlesTokenCancellation intermittently timed out on CI (the same push produced both green and red runs, and which of the two tests failed varied by OS). Root cause: RecordCancellationCleanup ran Thread.Sleep(CleanupDelayMilliseconds) = 5000ms of blocking work on a thread-pool thread. That both consumed most of the 15s WaitForAsync budget and starved the thread pool, delaying the timer-driven Task.Delay/CancelAsync callbacks the cancellation tests wait on. Under CI coverage instrumentation and slower, contended runners the ~7s happy path tipped past 15s. The cleanup duration is only there to simulate synchronous work; its length is irrelevant to what the tests assert. Decouple it from the (cancelled-anyway) Task.Delay constant and shorten it to 250ms via a new CleanupWorkMilliseconds. This removes the thread-pool starvation and drops the two tests from ~7s to ~2.4s, giving ample margin under the 15s timeout. The other tests in the class wait a fixed 6000ms after disposal, which still safely exceeds the shortened cleanup, so they are unaffected.
…proval EOL-resilient The PR moves the disposable helpers into ReactiveUI.Primitives, changing the public surface: AnonymousDisposable becomes ActionDisposable, EmptyDisposable exposes a static Instance, and DisposableBag/MutableDisposable/OnceDisposable/SwapDisposable are added. Regenerate the ApiApprovalTests.Primitives baselines (net8/9/10) to match. Also normalise line endings in CheckApproval before verifying so the approval no longer depends on the baseline and the generated API text sharing the same CRLF/LF convention across platforms.
Core and Async already alias `Lock` to System.Threading.Lock on .NET 9+ and to System.Object elsewhere via a project-level Using. Add the same alias to the Extensions project and remove the 37 redundant `#if NET9_0_OR_GREATER ... Lock ... #else ... object ... #endif` blocks across core, Extensions, and Async, replacing each with a single `private readonly Lock` field. Behaviour is unchanged (Lock on net9+, Monitor-on-object otherwise); the gates are just declared once now.
…bserver ObservableSubscribeExtensions.SubscribeCallbacks built its own DelegateObserver<T>, a hand-rolled copy of the core EmptyWitness<T> sink that SubscribeMixins.Subscribe already wraps delegates in (and which also has an inline fast path). Forward SubscribeCallbacks to the core Subscribe(onNext, onError, onCompleted) extension and delete the duplicated DelegateObserver plus its now-redundant test. Behaviour is unchanged; the unique SubscribeCallbacks name is kept so call sites that also import System.Reactive stay unambiguous.
|
glennawatson
approved these changes
May 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request makes significant updates to the
README.mdto document two new optional packages:ReactiveUI.Primitives.AsyncandReactiveUI.Primitives.Extensions. The changes provide detailed usage instructions, API summaries, migration guidance, and benchmarking results for these packages. The documentation now clearly distinguishes between the base package and these new optional packages, including their APIs, compatibility, and performance comparisons.Key documentation updates:
New package documentation and usage
ReactiveUI.Primitives.AsyncandReactiveUI.Primitives.Extensions, including installation instructions, namespace imports, supported target frameworks, and detailed API tables with usage examples. [1] [2] [3] [4] [5]Bridge and migration guidance
ReactiveUI.Extensions. [1] [2] [3]Benchmarks and validation
ReactiveUI.Extensions4.0.0 andReactiveUI.Extensions.Async4.0.0.Summary:
These changes ensure that the documentation fully covers the new async and extension helper packages, clarifies their relationship to the core library, and provides migration and performance guidance for users transitioning from older helper packages.