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

Follow .NET lifecycle: update codebase to net462 #3646

Merged
merged 20 commits into from
Jul 15, 2022

Conversation

Evangelink
Copy link
Member

@Evangelink Evangelink commented May 17, 2022

Description

Follow Microsoft .NET Framework component lifecycle policy by dropping support for .NET Framework older than 4.6.2.

Related issue

Replaces #3517 and fixes AB#1491257

@Evangelink Evangelink changed the title Update codebase to net462 (oldest supported version) Follow .NET lifecycle: update codebase to net462 May 18, 2022
@Evangelink Evangelink force-pushed the net45X-to-net462 branch 2 times, most recently from 2bb119d to 2f17d48 Compare May 23, 2022 11:31
@Evangelink Evangelink force-pushed the net45X-to-net462 branch 2 times, most recently from 9615564 to 2c57707 Compare June 24, 2022 11:00
@@ -13,7 +13,7 @@
<AssemblyName Condition=" '$(RuntimeIdentifier)' == 'win10-arm64' ">datacollector.arm64</AssemblyName>
<TargetFrameworks>netcoreapp2.1</TargetFrameworks>
<TargetFrameworks Condition=" '$(OS)' == 'WINDOWS_NT' ">$(TargetFrameworks);net472</TargetFrameworks>
Copy link
Member Author

Choose a reason for hiding this comment

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

Why do we have a different min TFM depending on windows or not? I'd like to document that in the project.

Copy link
Member

Choose a reason for hiding this comment

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

The windows code of datacollector is using something that is net472 specific, I don't remember what it is though, but you should be able to see if you downgrade. You can remove the condition for the different tfm on Linux, that probably happened because getting the whole thing build on linux was difficult in the past and I did not want to disrupt the linux build by changing it.

@nohwnd
Copy link
Member

nohwnd commented Jul 14, 2022

It might be a good idea to introduce a variable like MinNetFxVersion and use it in all projects so that we can have an easier time on the next bump of netfx. We could also do the same for netcoreapp and net.

@nohwnd WDYT?

You mean for TargetFrameworks in the csproj? I thought we have the defaults in Directory.Build.props, maybe I am wrong. But that is probably the best place for a baseline.

@Evangelink
Copy link
Member Author

It might be a good idea to introduce a variable like MinNetFxVersion and use it in all projects so that we can have an easier time on the next bump of netfx. We could also do the same for netcoreapp and net.
@nohwnd WDYT?

You mean for TargetFrameworks in the csproj? I thought we have the defaults in Directory.Build.props, maybe I am wrong. But that is probably the best place for a baseline.

Yes. Just so that it's easier to update.

scripts/build.ps1 Show resolved Hide resolved
scripts/verify-nupkgs.ps1 Show resolved Hide resolved
@@ -19,7 +19,7 @@ public class TraitCollection : IEnumerable<Trait>
internal const string TraitPropertyId = "TestObject.Traits";
private static readonly TestProperty TraitsProperty = TestProperty.Register(
TraitPropertyId,
#if !NET451
#if !NET462
Copy link
Member

Choose a reason for hiding this comment

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

this seems wrong. the condition said if not net451 before, so for net462 the condition body should be used, and else case should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in a follow-up PR.

@nohwnd nohwnd merged commit ada1268 into microsoft:main Jul 15, 2022
@Evangelink Evangelink deleted the net45X-to-net462 branch July 15, 2022 14:12
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.

2 participants