-
-
Notifications
You must be signed in to change notification settings - Fork 746
Partially revert "Drop .NET 6.0, default to .NET 10.0 and update dependencies" #976
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
Conversation
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.
Pull request overview
This PR partially reverts a previous change that dropped .NET 6.0 support, restoring multi-targeting to .NET 6.0, 8.0, and 10.0 for the ElectronNET library packages. The change maintains backward compatibility for users still on .NET 6.0 while supporting newer framework versions.
Key Changes:
- Restored .NET 6.0 as a target framework alongside .NET 8.0 and 10.0 for all library projects
- Downgraded Microsoft.Build.Utilities.Core from 18.0.2 to 17.3.2 to maintain compatibility with older MSBuild and Visual Studio versions
- Downgraded System.Drawing.Common and System.Text.Json packages from 10.0.1 to 8.0.x versions for compatibility with the multi-targeting approach
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ElectronNET/ElectronNET.csproj | Added net6.0 to TargetFrameworks, now targets net6.0, net8.0, and net10.0 |
| src/ElectronNET.Build/ElectronNET.Build.csproj | Downgraded Microsoft.Build.Utilities.Core to 17.3.2 and applied minor formatting improvements to MSBuild tasks |
| src/ElectronNET.AspNet/ElectronNET.AspNet.csproj | Added net6.0 to TargetFrameworks and introduced configuration-specific PropertyGroups for net6.0 builds |
| src/ElectronNET.API/ElectronNET.API.csproj | Added net6.0 to TargetFrameworks and downgraded System.Drawing.Common and System.Text.Json to 8.0.x versions |
Comments suppressed due to low confidence (1)
src/ElectronNET.AspNet/ElectronNET.AspNet.csproj:34
- The NoWarn settings in these configuration-specific PropertyGroups are redundant. Line 16 already defines the same NoWarn directive at the project level, which applies to all configurations and target frameworks. These duplicate conditional PropertyGroups should be removed to reduce maintenance burden and potential inconsistencies.
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net6.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net8.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net10.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net6.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net8.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net10.0|AnyCPU'">
<NoWarn>1701;1702;4014;CS4014;CA1416;CS1591</NoWarn>
</PropertyGroup>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eb33b8b to
0d732ce
Compare
|
I'm thankful that we share the same opinion on this one, even though I was not strongly opposed to the change (and keeping the most recent one for the sample projects is totally fine imho). As library makers we should not rush to remove things that are "out of support" - if there is no good reason (e.g., it blocks a way forward). I will bring this to a contributing guide. Our guideline should be to support all LTS .NET versions as long as there is no technical reason to discard it (e.g., an outdated version blocks / makes it difficult releasing a new feature). |
Yea. And as a library consumer - when you are not just experimenting, but have business debt and responsibility, like an app with millions of users - then you don't simply update that app to a newer .net version because MS says you should. You'll have to organize and conduct new beta testing before rolling it out and you always need to find the right time for that first. Or for example when you want to build apps for Samsung Smart TVs: When you want to target .net 8, then your app can only work on TVs from this year and later. If you want it to work on older models as well, you need to use .net6 - these models just won't get new .net versions, no matter what MS says 😉 |
The key points are:
Microsoft.Build.Utilities.Core. This will make Electron.NET stop working on older Visual Studio and MSBuild versions. There's are also no reasons to update it in the first placeI'd also like to note the MS saying "Out of support" has almost no practical meaning. I've never seen any bugs fixed in the same .net version which mattered to me. The bugs that all new .net versions have are much worse than mature .net versions which are declared as "out of support".
Also, MS have become more lazy in recent .net versions about downlevel support. From one day to another, out .net8 MAUI builds have stopped building on CI with a message that it's unsupported and we should update to .net 10 - only few weeks after .net 10 has been released - that's insane. It's not MS to decide at what time we will update our software - and definitely not within a timeframe of 4 weeks.
I know that many others are thinking the same way and therefore we should not drop support for older .net versions when we don't have a better reason than some MS support declaration.