-
Notifications
You must be signed in to change notification settings - Fork 287
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 netcoreapp2.1 from source #2350
Conversation
...oft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Channel/TransmissionTest.cs
Outdated
Show resolved
Hide resolved
BASE/Test/Microsoft.ApplicationInsights.Test/Standalone/AppInsightsStandaloneTests.cs
Show resolved
Hide resolved
@@ -34,6 +34,9 @@ | |||
|
|||
<ItemGroup Condition="'$(TargetFramework)' == 'netcoreapp3.1'"> | |||
<PackageReference Include="Azure.Core" Version="1.14.0" /> | |||
|
|||
<PackageReference Include="Microsoft.AspNetCore.Hosting" Version="2.2.0" /> |
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.
suggest using the 2.1*
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.
why 2.1 here as opposed to latest?
I have no problem with the downgrade, just trying to understand :)
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.
if its testing, yes you can use latest.
#if NETCOREAPP2_1 | ||
Assert.IsTrue((float)payload["Max"] >= 30); | ||
#elif NETCOREAPP3_1 | ||
#if NETCOREAPP |
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.
If we are planning to cover .NET 5.0, .NET 6.0+, then better to include NET
along with NETCOREAPP
. Ref: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives#conditional-compilation
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.
NET
and NETCOREAPP
both cover the same frameworks.
In my opinion, NET
sounds ambiguous between the older NetFrameworks (4.5 - 4.8).
I would prefer to use NETCOREAPP
to differentiate.
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.
since NETCOREAPP covers NET5.0 and higher, this is fine i think.
Fix Issue #2251
Changes
#TODO
Checklist
For significant contributions please make sure you have completed the following items:
The PR will trigger build, unit tests, and functional tests automatically. Please follow these instructions to build and test locally.
Notes for authors:
Notes for reviewers:
/AzurePipelines run
will queue all builds/AzurePipelines run <pipeline-name>
will queue a specific build