-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove polyfill packages #2057
Comments
I'm lead to believe that removing dependencies is a breaking change (in fact, I hit this today in #2003 due to changes to Microsoft.Extensions.TimeProvider.Testing dropping seemingly redundant dependencies), so if that's the case this isn't something we'd do until there's a reason for a Polly v9 if we did. |
In the case here I am referring to polyfill package's ie ones which are natively provided by the framework without it needing to be explicitly added hence I don't see it as a breaking change. |
Yes I understand that, and this comment is what lead me to my comment: dotnet/extensions#5058 (review) And also the fact that it actually broke the build in #2056 when I updated to Microsoft.Extensions.TimeProvider.Testing 8.4.0 without me adding the reference. |
I am still talking about polyfill package's and as such if a client is using System.Diagnostic.DiagnosticSource in their application/library it will still build as the framework will provide the library and likely even provide a newer version. |
@martintmk let me submit a PR tomorrow my time showing the change as your sample code is not quite what I am intending. |
Issue has been addressed already as part of the 8.4.0 release which has occured, hence closing issue |
Is your feature request related to a problem? Please describe.
I want to minimise dependencies in my project by utilising framework dependencies wherever possible
Describe the solution you'd like
In Polly.Extensions System.Diagnostics.DiagnosticSource can be provided via the framework ie net 6/8 as such conditions should be placed on it so that it is only included for the other frameworks. Other dependencies can be optimised.
Describe alternatives you've considered
Accept the additional dependency
Additional context
n/a
The text was updated successfully, but these errors were encountered: