Skip to content

Restore named timer BatchIf overload compatibility - #1189

Open
dwcullop wants to merge 2 commits into
mainfrom
u/dacullop/main/batchif-timer-compatibility
Open

dwcullop wants to merge 2 commits into
mainfrom
u/dacullop/main/batchif-timer-compatibility

Conversation

@dwcullop

Copy link
Copy Markdown
Member

Summary

Restores source compatibility for BatchIf calls that pass a named timer argument (with an optional scheduler) but omit initialPauseState. A compatibility overload is added so those call sites bind again; the batch starts unpaused, matching the previous behavior.

Also documents the overload's initial pause state in the cache operator instructions.

Validation

Prior validation already passed: the solution built across all seven target frameworks, and the focused BatchIf test suite passed. New API surface is captured in the approved API verification files for .NET 8 and .NET 9.

Fixes #1175

@dwcullop

Copy link
Copy Markdown
Member Author

P2: The new timer overload breaks an existing positional call.

Location: ObservableCacheEx.BatchIf.cs, lines 47-49.

source.BatchIf(pause, null, scheduler) compiles against the base commit, selecting the overload with a nullable timeout. Against this PR it fails with CS0121, because the new IObservable<Unit>? parameter and the existing TimeSpan? parameter both accept the positional null.

Suggested fix: change the added overload shape so named timer calls remain available without introducing another candidate for the existing positional nullable-timeout call. This requires an API-shape decision rather than a null check in the implementation, since resolution fails before any method runs. Include compile-time compatibility coverage for both named-timer calls and the existing BatchIf(pause, null, scheduler) form. Requiring downstream callers to add a cast would leave this as a source-breaking change rather than a compatibility restoration.

@@ -0,0 +1,73 @@
// Copyright (c) 2011-2026 Roland Pheasant. All rights reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Based on the rest of the codebase, we generally don't test operator overloads that are just a no-logic alias of another. I don't see why we should start here.

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.

[Bug]: BatchIf no longer accepts an existing named-timer call without initialPauseState

2 participants