NightOwl888 commented on code in PR #1222:
URL: https://github.com/apache/lucenenet/pull/1222#discussion_r2553874167
##########
.github/workflows/Generate-TestWorkflows.ps1:
##########
@@ -284,6 +287,12 @@ jobs:
uses: actions/setup-dotnet@v5
with:
dotnet-version: '$DotNet9SDKVersion'
+ if: `${{ startswith(matrix.framework, 'net9.') }}
+
+ - name: Setup .NET 10 SDK
+ uses: actions/setup-dotnet@v5
+ with:
+ dotnet-version: '$DotNet10SDKVersion'
Review Comment:
Note that we got some feedback about how the `setup-dotnet` task works,
which was different from the assumptions made when this task was written:
https://github.com/actions/setup-dotnet/issues/679#issuecomment-3550932760
As pointed out, the latest SDK is always installed when specifying a lower
one.
While this configuration will work, it would be more explicit (and faster)
if this were also conditional. But keep in mind, we also need to ensure it is
installed for building .NET Framework or .NET Standard. In this case, we are
building test projects, though, so the matrix only includes executable target
frameworks. Therefore, we can do:
```yml
- name: Setup .NET 10 SDK
uses: actions/setup-dotnet@v5
with:
dotnet-version: '$DotNet10SDKVersion'
if: `${{ startswith(matrix.framework, 'net4') ||
startswith(matrix.framework, 'net10.') }}
```
##########
.github/workflows/Generate-TestWorkflows.ps1:
##########
@@ -65,14 +65,16 @@ param(
[string]$RepoRoot = (Split-Path (Split-Path $PSScriptRoot)),
- [string[]]$TestFrameworks = @('net9.0','net8.0','net472','net48'), #
targets under test: net8.0, net8.0, netstandard2.0, net462
+ [string[]]$TestFrameworks = @('net10.0',
'net9.0','net8.0','net472','net48'), # targets under test: net10.0, net8.0,
net8.0, netstandard2.0, net462
Review Comment:
It is unlikely anything will slip through if we eliminate `net9.0` here,
since these tests are just to gatekeep the PRs in a fast and efficient way. We
don't bother with macOS or slow tests here, either.
We always run Azure DevOps tests before a release, which include all of the
test frameworks, operating systems, and run the full set of tests.
But we need `net10.0` because we should have some coverage on that target
before PRs are merged.
--
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]
