-
Notifications
You must be signed in to change notification settings - Fork 785
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 net5.0 from Zipkin and replace with net6.0 #3222
Remove net5.0 from Zipkin and replace with net6.0 #3222
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3222 +/- ##
==========================================
- Coverage 85.74% 85.44% -0.30%
==========================================
Files 260 260
Lines 9366 9366
==========================================
- Hits 8031 8003 -28
- Misses 1335 1363 +28
|
@@ -1,15 +1,15 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
<PropertyGroup> | |||
<!-- OmniSharp/VS Code requires TargetFrameworks to be in descending order for IntelliSense and analysis. --> | |||
<TargetFrameworks>net5.0;netstandard2.0;net462;</TargetFrameworks> | |||
<TargetFrameworks>net6.0;netstandard2.0;net462;</TargetFrameworks> |
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.
@cijothomas @alanwest @reyang @utpilla
Was just looking at this while investigating something else. It didn't occur to me at the time, but we kind of introduced an issue doing this.
Previously we had a net5.0
target and code like this:
#if NET5_0_OR_GREATER
using var response = this.httpClient.Send(request, CancellationToken.None);
#else
That became:
#if NET6_0_OR_GREATER
using var response = this.httpClient.Send(request, CancellationToken.None);
#else
Users on .NET 5 upgrading to latest will still compile, they will just bind to the netstandard2.1
target. However, they have lost the optimization. So for them, upgrading was a perf hit/carried unforeseen consequences.
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.
Good find. Lesson is that in the future we need to align any drop of a target framework with the actual EOL date of the framework and consider which approach we're going to adopt here #3448
Part of #3147