-
Notifications
You must be signed in to change notification settings - Fork 652
Add .NET 10 target, #1219 #1222
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
base: master
Are you sure you want to change the base?
Conversation
|
Note that this PR reverts the hardcoded .NET 9 SDK since a new version is out that resolves that issue. |
| <TargetFrameworks>net8.0</TargetFrameworks> | ||
| <RollForward Condition=" $(TargetFramework.StartsWith('net8.')) ">Major</RollForward> | ||
| <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks> | ||
| <RollForward>Major</RollForward> |
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.
We need this to be conditional, since we should only roll forward from the highest version we support to any future version of .NET.
Technically, we could do it this way, but this is the largest package by far and packing up net9.0 (with only 6 months of support left) seems like a non-starter. So, IMO, we should remove net9.0 and make both net8.0 and net10.0 roll forward to future versions, which would include net9.0 rolled forward from net8.0.
<TargetFrameworks>net10.0;net8.0</TargetFrameworks>
<RollForward Condition=" $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) ">Major</RollForward>|
|
||
| <IsPublishable>false</IsPublishable> | ||
| <IsPublishable Condition="'$(TargetFramework)' == 'net8.0'">true</IsPublishable> | ||
| <IsPublishable>true</IsPublishable> |
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 we publish on .NET Framework for the solution, this must be conditional, since .NET Tools only work on .NET Core.
<IsPublishable Condition="' $(TargetFramework.StartsWith('net8.')) Or $(TargetFramework.StartsWith('net10.')) '">true</IsPublishable>|
|
||
| <PropertyGroup> | ||
| <LangVersion>11.0</LangVersion> | ||
| <LangVersion>14.0</LangVersion> |
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.
Typically, we only bump LangVersion if we need features from the language due to the knock-on effects that it could create. And we wouldn't need to rename all of the field fields in this PR.
Let's split this out into a separate issue if we need to do it at all.
One issue with this is that it immediately means that the latest tooling is required that supports .NET 10 to work on Lucene.NET. IMO, it would be better for contributors if we supported VS2022/.NET8/.NET9 so we don't leave contributors that may not have the time or space to install new tools behind. We can do this by staying on an older LangVersion and making the net10.0 target conditional depending on the version of Visual Studio installed. This should be fine because issues on a specific TFM are rare and any issues that arise would be caught by CI.
An example of this approach can be found in ICU4N:
<TargetFrameworks Condition="$(VisualStudioVersion) >= 16.10">net9.0;net8.0</TargetFrameworks>
<TargetFrameworks>$(TargetFrameworks);netstandard2.0;net462;net451;net40</TargetFrameworks>At one point, I think we had net6.0 requiring $(VisualStudioVersion) >= 16.10 and net5.0 always enabled. We can do something similar using the condition $(VisualStudioVersion) >= 18.0 for net10.0 now.
Of course, there is another level to this in the tests, since we only compile the .NET Framework tests on Windows.
<TargetFrameworks Condition=" '$(TestFrameworks)' == 'true' AND $([MSBuild]::IsOsPlatform('Windows')) ">$(TargetFrameworks);net48;net472</TargetFrameworks>And in the case of Lucene.NET, we only have it defaulted to run tests for a single target framework, so that would need to be conditional depending on the tooling.
Add .NET 10 target
Fixes #1219