NightOwl888 commented on code in PR #930:
URL: https://github.com/apache/lucenenet/pull/930#discussion_r1522552973
##########
src/dotnet/tools/lucene-cli/lucene-cli.csproj:
##########
@@ -24,22 +24,22 @@
<Import Project="$(SolutionDir).build/nuget.props" />
<PropertyGroup>
- <TargetFrameworks>net7.0;net6.0</TargetFrameworks>
- <RollForward Condition=" $(TargetFramework.StartsWith('net7.'))
">Major</RollForward>
+ <TargetFrameworks>net8.0;net6.0</TargetFrameworks>
+ <RollForward Condition=" $(TargetFramework.StartsWith('net8.'))
">Major</RollForward>
Review Comment:
Before we can merge this, the experimentation with roll-forward will need to
be done on .NET 8 to ensure that it works. Unless you want to use the preview 2
of .NET 9, but I have doubts that would be a valid test because the behavior
might change before the release.
That said, it might make sense to check it out on .NET 9 to see if there are
any behavioral differences in roll-forward that may need to be pointed out in
the docs that differ from how it worked in .NET 8.
##########
Directory.Build.targets:
##########
@@ -22,45 +22,45 @@
<Import Project=".build/dependencies.props"
Condition="Exists('.build/dependencies.props')" />
- <!-- Features in .NET 6.x and .NET 7.x only -->
- <PropertyGroup Condition=" $(TargetFramework.StartsWith('net6.')) Or
$(TargetFramework.StartsWith('net7.')) ">
+ <!-- Features in .NET 6.x, .NET 7.x, and .NET 8.x only -->
+ <PropertyGroup Condition=" $(TargetFramework.StartsWith('net6.')) Or
$(TargetFramework.StartsWith('net7.')) Or
$(TargetFramework.StartsWith('net8.')) ">
<DefineConstants>$(DefineConstants);FEATURE_SPANFORMATTABLE</DefineConstants>
<DefineConstants>$(DefineConstants);FEATURE_RANDOM_NEXTINT64_NEXTSINGLE</DefineConstants>
Review Comment:
Let's swap these to keep them all in lexographical order.
##########
src/dotnet/tools/lucene-cli/.gitignore:
##########
@@ -0,0 +1,2 @@
+work/
Review Comment:
Not sure whether it is worth it to maintain multiple .gitignore files. Can
we move this up to the root .gitignore instead?
##########
TestTargetFramework.props:
##########
@@ -66,7 +67,7 @@
<PropertyGroup Label="Warnings to be Disabled in Test Projects">
<!-- We purposely test on EoL frameworks for testing netstandard2.1, but
we want to keep this warning in production code. -->
<CheckEolTargetFramework>false</CheckEolTargetFramework>
Review Comment:
We should make this conditional on `net5.0`, since it makes sense to be
notified of out of support frameworks higher than `net5.0`.
##########
TestTargetFramework.props:
##########
@@ -30,22 +30,23 @@
<!--<TargetFramework>net48</TargetFramework>-->
<!--<TargetFramework>net5.0</TargetFramework>-->
<!--<TargetFramework>net6.0</TargetFramework>-->
- <TargetFramework>net7.0</TargetFramework>
+ <!--<TargetFramework>net7.0</TargetFramework>-->
Review Comment:
Since this won't be a valid test TFM, we can remove it to avoid maintenance
confusion. If someone wanted to test on `net7.0` for some odd reason after it
is out of support, they could simply add this themselves.
--
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]
