NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2553856055
##########
Directory.Build.props:
##########
@@ -26,7 +26,7 @@
</PropertyGroup>
<PropertyGroup>
- <LangVersion>11.0</LangVersion>
+ <LangVersion>14.0</LangVersion>
Review Comment:
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`:
```xml
<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.
```xml
<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.
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.'))
">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
Review Comment:
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`.
```xml
<TargetFrameworks>net10.0;net8.0</TargetFrameworks>
<RollForward Condition=" $(TargetFramework.StartsWith('net8.')) Or
$(TargetFramework.StartsWith('net10.')) ">Major</RollForward>
```
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,11 +24,10 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net8.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net8.'))
">Major</RollForward>
+ <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
+ <RollForward>Major</RollForward>
- <IsPublishable>false</IsPublishable>
- <IsPublishable Condition="'$(TargetFramework)' ==
'net8.0'">true</IsPublishable>
+ <IsPublishable>true</IsPublishable>
Review Comment:
Since we publish on .NET Framework for the solution, this *must* be
conditional, since .NET Tools only work on .NET Core.
```xml
<IsPublishable Condition="' $(TargetFramework.StartsWith('net8.')) Or
$(TargetFramework.StartsWith('net10.')) '">true</IsPublishable>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
