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

v6.0: Rationalise Execute()/ExecuteAsync() overloads #425

Closed
reisenberger opened this issue Mar 30, 2018 · 2 comments
Closed

v6.0: Rationalise Execute()/ExecuteAsync() overloads #425

reisenberger opened this issue Mar 30, 2018 · 2 comments

Comments

@reisenberger
Copy link
Member

reisenberger commented Mar 30, 2018

Polly historically includes a large number of .Execute(...) or .ExecuteAsync(...) overloads, 20-40 per policy type (sync vs async, non-generic vs generic). There are then a comparable number of ExecuteAndCapture(...) overloads.

Polly v6.0 proposes rationalising the number of execute overloads by removing some anomalous cases. Certain overloads dating back several years take an input parameter, for example Context, but do not pass this parameter to the delegate to execute (example). This defies expectations in not providing a pattern where the input parameters passed to the overload match the input parameters taken by the executed delegate. (Why do they exist? They date from pre- current maintainers and could have obscure uses in passing the Context to an onRetry policy hook but not the delegate.)

Polly v6.0 proposes rationalising the .Execute/Async/AndCapture() overloads by removing these and a few other minor variants.

All affected overloads have already been omitted from the Polly execution interfaces available since Polly v5.2.0, ie for the last 8 releases / 9 months. Those already using the interfaces will experience no change.

v5.9.0 will pre-advertise the deprecations with [ObsoleteAttribute()], per semver. v6.0 would push through the changes as breaking changes.

EDITED: to pin linked example by linking to specific commit.

@reisenberger reisenberger added this to the v5.9.0 milestone Mar 30, 2018
@reisenberger reisenberger changed the title v5.9.0 / v6.0: Rationalise Execute()/ExecuteAsync() overloads v6.0: Rationalise Execute()/ExecuteAsync() overloads Mar 30, 2018
reisenberger added a commit that referenced this issue Apr 9, 2018
+ #424 (fixes #419) Allow `Timeout.InfiniteTimeSpan` for TimeoutPolicy
+ #425 Rationalise Execute()/ExecuteAsync() overloads: pre-advise ([per SemVer](https://semver.org/#how-should-i-handle-deprecating-functionality)) expected breaking changes for v6.0
+ #426 Better name ExecutionKey and ExecutionGuid: pre-advise ([per SemVer](https://semver.org/#how-should-i-handle-deprecating-functionality)) expected breaking changes for v6.0
+ Add `Context()` public noargs ctor
+ Minor corrections to method visibility
@reisenberger
Copy link
Member Author

Polly v5.9.0 has now been published. It deprecates the overloads scheduled for removal (example).

Polly v6.0 will removed the deprecated overloads entirely.

@reisenberger
Copy link
Member Author

Documentation published to wiki

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant