Ready for net10.0 support release #251
Conversation
WalkthroughRepository-wide upgrade to .NET 10.0, widespread package/tool version bumps, many project-file warning-suppression adjustments, added Sample4-10 coverage data, several small F# refactors (range → seq) and formatting/metadata changes; build/test paths and scripts updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🪛 LanguageToolReleaseNotes.md[grammar] ~7-~7: Ensure spelling is correct (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
AltCover.Monitor.Tests/MonitorTest.fs (2)
92-99: Clarify the bounds derivation approach.The comments suggest bounds are "derived from discovered expectations," but most values are hardcoded:
minExpectedCodeis fixed at 112 (not derived fromexpectedCodes)expectedBranchesis hardcoded as(24, 38)(not derived fromexpect)- Only
maxExpectedCodeWithSlackis actually derived from the dataAdditionally, the comment at line 95-96 states that 112 is "far less than" the actual minimum in
expectedCodes, but doesn't explain why such a permissive lower bound is appropriate. This could allow tests to pass even when code coverage unexpectedly drops significantly.Consider either:
- Updating comments to accurately reflect that these are historical safety bounds with only the upper code limit being data-driven, or
- Actually deriving all bounds from the expectations data if that was the intent
105-106: Consider destructuring for readability.Using
fst expectedBranchesandsnd expectedBranchesis less readable than destructuring the tuple. Consider:let expectedBranches = (24, 38) // as historically observed + let (minBranch, maxBranch) = expectedBranches test' <@ code >= minExpectedCode && code <= maxExpectedCodeWithSlack - && branch > fst expectedBranches - && branch < snd expectedBranches + && branch > minBranch + && branch < maxBranch @>Build/Build.fsproj (1)
3-4: Remove the empty PropertyGroup.The empty
PropertyGroupserves no purpose and should be removed to keep the project file clean.Apply this diff:
- <PropertyGroup> - </PropertyGroup> -AltCover.Recorder2.Tests/AltCover.Recorder2.Tests.fsproj (1)
29-30: Consider fixing typo in framework condition.Line 29 contains
'ne472'which appears to be a typo for'net472'. While this doesn't affect the current single-targeted net10.0 build, it reduces code clarity. Consider correcting this for consistency with line 30.- <Compile Include="..\AltCover.Recorder.Tests\Program.fs" Link="Program.fs" Condition="'$(TargetFramework)' != 'ne472'" /> + <Compile Include="..\AltCover.Recorder.Tests\Program.fs" Link="Program.fs" Condition="'$(TargetFramework)' != 'net472'" />Build/Setup.fsproj (1)
3-18: LGTM! Warning suppression consolidation.The removal of NoWarn and MSBuildWarningsAsMessages entries aligns with the PR's broader effort to consolidate warning suppressions. The empty PropertyGroup could optionally be removed, but retaining it may be intentional for future configuration.
If desired, the empty PropertyGroup at lines 3-4 can be removed:
- <PropertyGroup> - </PropertyGroup> - <ItemGroup>Build/targets.fs (1)
2030-2055: Fallback to “.net10.0.xml” report namesThe Replace(".xml", ".net10.0.xml") fallback is fine, but a tiny nit: Replace acts on the whole string. If “.xml” appeared earlier in the path, you’d get a surprising rename. Using Path.GetFileName/ChangeExtension to rebuild just the tail would be safer. Optional.
- else report.Replace(".xml", ".net10.0.xml") + else + let dir = System.IO.Path.GetDirectoryName(report) + let name = System.IO.Path.GetFileNameWithoutExtension(report) + ".net10.0.xml" + System.IO.Path.Combine(dir, name)Also applies to: 2076-2082
AltCover.Engine.Tests/Runner.fs (1)
2082-2086: net10.0 path bump looks right; consider de-hardcoding the TFM for future releasesAvoid hard-coding "net10.0" so this test won’t need edits on the next TFM bump. Probe the binaries folder for the highest netX directory and fall back to net10.0.
#else - let path = - Path.Combine( - SolutionRoot.location, - "_Binaries/Sample12/Debug+AnyCPU/net10.0/Sample12.dll" - ) + let binaries = Path.Combine(SolutionRoot.location, "_Binaries/Sample12/Debug+AnyCPU") + let tfm = + Directory.EnumerateDirectories(binaries) + |> Seq.map Path.GetFileName + |> Seq.filter (fun n -> n.StartsWith("net", StringComparison.Ordinal)) + |> Seq.sortDescending + |> Seq.tryHead + |> Option.defaultValue "net10.0" + let path = Path.Combine(binaries, tfm, "Sample12.dll") #endifIf Sample12’s project hasn’t been updated to include net10.0, please confirm and adjust.
AltCover.Avalonia/MainWindow.fs (1)
496-506: Prefer for-to loop and cache length (minor nit)The seq wrapper is fine; a simple for-to with a cached length avoids building a sequence and is slightly clearer.
- for l in seq { 1 .. lines.Length } do + let n = lines.Length + for l = 1 to n doAltCover.Engine.Tests/AltCover.Engine.Tests.fsproj (2)
84-84: New resource Sample4-10.native.jsonOK to add alongside the existing Sample4.native.json. If this is only used for net10.0 scenarios, consider adding a TargetFramework condition to keep assets minimal.
136-136: System.IO.Compression package reference scopeOn net10.0 this assembly comes from the platform; you can scope the package to net472 to avoid redundant restore on modern TFMs.
- <PackageReference Include="System.IO.Compression" /> + <PackageReference Include="System.IO.Compression" Condition="'$(TargetFramework)' == 'net472'" />AltCover.Engine.Tests/Tasks.fs (1)
918-926: Path switch to net10.0 is fine; make it TFM-agnostic like aboveMirror the dynamic TFM detection to avoid future edits when updating TFMs.
-#if !NET472 - "_Binaries/Sample4/Debug+AnyCPU/net10.0" - ) +#if !NET472 + let binaries = Path.Combine(SolutionRoot.location, "_Binaries/Sample4/Debug+AnyCPU") + let tfm = + Directory.EnumerateDirectories(binaries) + |> Seq.map Path.GetFileName + |> Seq.filter (fun n -> n.StartsWith("net", StringComparison.Ordinal)) + |> Seq.sortDescending + |> Seq.tryHead + |> Option.defaultValue "net10.0" + Path.Combine(binaries, tfm) + ) #else "_Binaries/Sample4/Debug+AnyCPU/net472" ) #endifAltCover.Visualizer/Visualizer.fs (1)
510-527: Tiny readability/perf tweak: use for-to and avoid seq creationSemantics unchanged; a direct for-to with cached LineCount is a touch leaner.
- for l in seq { 1 .. buff.LineCount } do + let n = buff.LineCount + for l = 1 to n do
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ThirdParty/BlackFox.CommandLine.dllis excluded by!**/*.dll
📒 Files selected for processing (83)
.config/dotnet-tools.json(3 hunks).gitignore(1 hunks)AltCover.Api.Tests/AltCover.Api.Tests.fsproj(1 hunks)AltCover.Async/AltCover.Async.csproj(1 hunks)AltCover.Avalonia/AltCover.Avalonia.fsproj(1 hunks)AltCover.Avalonia/MainWindow.fs(1 hunks)AltCover.Base/AltCover.Base.csproj(0 hunks)AltCover.DataCollector/AltCover.DataCollector.csproj(0 hunks)AltCover.DotNet/DotNet.fsi(1 hunks)AltCover.Engine.Tests/AltCover.Engine.Tests.fsproj(3 hunks)AltCover.Engine.Tests/Locations.fs(1 hunks)AltCover.Engine.Tests/Main.fs(1 hunks)AltCover.Engine.Tests/Program.fs(1 hunks)AltCover.Engine.Tests/Runner.fs(1 hunks)AltCover.Engine.Tests/Tasks.fs(1 hunks)AltCover.Engine.Tests/Visitor.fs(5 hunks)AltCover.Engine/AltCover.Engine.fsproj(1 hunks)AltCover.Engine/CommandLine.fs(1 hunks)AltCover.Engine/NativeJson.fs(1 hunks)AltCover.Engine/ProgramDatabase.fs(1 hunks)AltCover.Fake.DotNet.Testing.AltCover/AltCover.Fake.DotNet.Testing.AltCover.fsproj(2 hunks)AltCover.Fake/AltCover.Fake.fsproj(2 hunks)AltCover.FontSupport/AltCover.FontSupport.csproj(1 hunks)AltCover.Monitor.Tests/AltCover.Monitor.Tests.fsproj(1 hunks)AltCover.Monitor.Tests/MonitorTest.fs(2 hunks)AltCover.Monitor/AltCover.Monitor.csproj(0 hunks)AltCover.Recorder.Tests/AltCover.Recorder.Tests.fsproj(1 hunks)AltCover.Recorder/AltCover.Recorder.csproj(1 hunks)AltCover.Recorder2.Tests/AltCover.Recorder2.Tests.fsproj(1 hunks)AltCover.TestData/AltCover.Tests.xml(1 hunks)AltCover.TestData/AltCoverFSharpTypes.n3.xml(1 hunks)AltCover.TestData/OpenCoverForPester.coverlet.xml(1 hunks)AltCover.TestData/Sample4-10.native.json(1 hunks)AltCover.UICommon/CoverageFile.fs(1 hunks)AltCover.ValidateGendarmeEmulation/AltCover.ValidateGendarmeEmulation.fsproj(1 hunks)AltCover.Visualizer.Tests/AltCover.Visualizer.Tests.fsproj(1 hunks)AltCover.Visualizer/AltCover.Visualizer3.fsproj(1 hunks)AltCover.Visualizer/Persistence.fs(2 hunks)AltCover.Visualizer/Visualizer.fs(1 hunks)AltCover/AltCover.fsproj(0 hunks)Build/Build.fsproj(1 hunks)Build/Directory.Build.props(1 hunks)Build/DriveApi.fsproj(1 hunks)Build/Pester.Tests.ps1(7 hunks)Build/Setup.fsproj(1 hunks)Build/merge-coverage.ps1(1 hunks)Build/msbuildtest.proj(2 hunks)Build/targets.fs(54 hunks)Demo/Echo/Echo/Echo.csproj(1 hunks)Demo/Echo/Test/Test.csproj(1 hunks)Demo/MultiTest/Test1/Test1.fsproj(1 hunks)Demo/MultiTest/Test2/Test2.fsproj(1 hunks)Directory.Build.props(1 hunks)Directory.Build.targets(1 hunks)Directory.Packages.props(1 hunks)FakeForAltCoverBuild.slnx(1 hunks)OutputBuildProps.props(1 hunks)README.md(3 hunks)RegressionTesting/SoakTest1/SoakTest1.fsproj(1 hunks)RegressionTesting/issue20/xunit-tests/xunit-tests.xml(1 hunks)ReleaseNotes.md(1 hunks)Samples/Sample1/Sample1.csproj(1 hunks)Samples/Sample10/Sample10.csproj(1 hunks)Samples/Sample12/Sample12.csproj(1 hunks)Samples/Sample13/Sample13.fsproj(1 hunks)Samples/Sample14/Sample14/Sample14.csproj(1 hunks)Samples/Sample16/Test/Test/Test.csproj(1 hunks)Samples/Sample17/Sample17.fsproj(1 hunks)Samples/Sample18/Sample18.fsproj(1 hunks)Samples/Sample2/Sample2.fsproj(1 hunks)Samples/Sample21/Sample21.csproj(1 hunks)Samples/Sample23/Sample23.csproj(1 hunks)Samples/Sample24/Sample24.csproj(1 hunks)Samples/Sample26/Sample26.fsproj(1 hunks)Samples/Sample27/Sample27.fsproj(1 hunks)Samples/Sample30/Sample30.fsproj(1 hunks)Samples/Sample32/Sample32.csproj(1 hunks)Samples/Sample4/Sample4.fsproj(1 hunks)Samples/Sample4/Sample4LongForm.fsproj(1 hunks)Samples/Sample6/Sample6.fsproj(1 hunks)Samples/Sample7/Sample7.fsproj(1 hunks)Samples/Sample8/Sample8.csproj(1 hunks)global.json(1 hunks)
💤 Files with no reviewable changes (4)
- AltCover.DataCollector/AltCover.DataCollector.csproj
- AltCover/AltCover.fsproj
- AltCover.Base/AltCover.Base.csproj
- AltCover.Monitor/AltCover.Monitor.csproj
🧰 Additional context used
🪛 LanguageTool
ReleaseNotes.md
[grammar] ~7-~7: Ensure spelling is correct
Context: ... # (Indori series release 2) * Net10.0 support * Fix issue #238 - some symbol errors caus...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
README.md
[style] ~112-~112: Consider using “incomplete” to avoid wordiness.
Context: ...has no ignore option) if the build is not complete and thus those expected assemblies are ...
(NOT_ABLE_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
README.md
77-77: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: windows
🔇 Additional comments (89)
Samples/Sample30/Sample30.fsproj (1)
4-4: Framework version update is correct and aligned with PR objectives.The change from
net9.0tonet10.0is straightforward and aligns with the systematic repository-wide migration to .NET 10.0 described in the PR objectives. The dual-target support (net472 and net10.0) is maintained as intended.Samples/Sample24/Sample24.csproj (1)
4-4: LGTM!The target framework update from .NET 9.0 to .NET 10.0 is correct and aligns with the PR objectives. The conditional NUnit version logic is preserved correctly—.NET Framework 4.7.2 continues to use the pinned 3.14.0 version while .NET 10.0 uses the default.
Samples/Sample1/Sample1.csproj (1)
5-5: Framework version upgrade to .NET 10.0 looks good.The multi-targeting pattern is preserved (net10.0;net20), maintaining backward compatibility while adding .NET 10.0 support as intended by the PR.
Please verify that .NET 10.0 is available in your build environment and that the project builds successfully with this target framework. Since my knowledge is current through March 2025, I want to confirm .NET 10.0 support readiness as of November 2025.
AltCover.FontSupport/AltCover.FontSupport.csproj (2)
8-8: Verify build impact of NoWarn narrowing before merge.The NoWarn attribute has been narrowed from "1701;1702;8002" to only "8002", which removes suppressions for CS1701 and CS1702 (assembly binding redirect warnings). The AI summary indicates this is intentional for observability, but removing previously-suppressed warnings can expose build failures or noise.
Please verify that:
- Warnings CS1701 and CS1702 no longer occur or are acceptable
- The build passes cleanly without these suppressions
- This change is consistent across related projects in the PR
4-4: Clarify whether net10.0 target framework should be added.The PR objective is "Ready for net10.0 support release," but this project's TargetFrameworks remain "netstandard2.0;net472" with no net10.0 target. Please confirm whether:
- This library intentionally does not target net10.0 (e.g., due to compatibility constraints with dependencies)
- net10.0 should be added to the targets as part of the PR scope
Samples/Sample18/Sample18.fsproj (1)
3-3: LGTM! Framework update is timely and correct.The update to .NET 10.0 aligns perfectly with the official GA release on November 11, 2025, making this PR well-timed for production use. As an LTS release with three years of support, this provides a stable foundation for the sample projects.
Build/Pester.Tests.ps1 (4)
45-47: LGTM! Framework paths updated consistently.The binary paths for both output and input directories have been correctly updated from net9.0 to net10.0.
72-72: Framework parameter updated correctly.The
--frameworkparameter passed todotnet testhas been properly updated to net10.0.
82-82: Path updated in error handling test.The framework version in the output path has been correctly updated. This change doesn't affect the error handling logic being tested.
378-378: Framework paths updated consistently in conversion tests.All assembly paths and expected path replacements have been correctly updated to net10.0. The test data compatibility pattern (replacing net6.0 references with net10.0 at runtime on lines 402-403 and 439-440) is handled correctly - this allows test data files to remain version-agnostic while testing against the current framework version.
Also applies to: 403-403, 415-415, 440-440
AltCover.DotNet/DotNet.fsi (1)
78-110: Formatting change looks clean.Indentation inside the TRACE cheat-mode module is now consistent and easier to scan, with no functional impact detected.
Samples/Sample16/Test/Test/Test.csproj (1)
4-4: Framework update and package compatibility verified — no issues found.xunit.runner.visualstudio 2.4.3 is compatible with net10.0 per NuGet. The framework version update is correct and the test infrastructure is properly aligned with the broader migration. xUnit v3 offers explicit net10.0 support as a future enhancement, but the current setup is stable and requires no changes.
AltCover.Monitor.Tests/MonitorTest.fs (2)
19-19: LGTM! Path updated for .NET 10.0.The test resource path has been correctly updated to reference the .NET 10.0 version of the coverage XML file, consistent with this PR's objective.
108-114: LGTM! Error message formatting is clear and comprehensive.The updated error message properly includes all relevant debugging information: the XML path, actual results, and the expected bounds for both codes and branches.
Samples/Sample6/Sample6.fsproj (2)
8-10: LGTM: Condition updated consistently.The Debug configuration condition correctly references the new net10.0 target framework, maintaining consistency with the TargetFrameworks property.
4-4: Framework update to .NET 10.0 is approved.The update from net9.0 to net10.0 while maintaining net472 support is correct and consistent. FSharp.Core latest versions support .NET 10.0, so the project will build successfully. Ensure your build environment has the .NET 10.0 SDK installed.
AltCover.Engine/CommandLine.fs (1)
497-502: System.Reflection removal is safe and the SuppressMessage target update is correct.The removal of the System.Reflection import does not break anything. The only reflection usage in the file is
Type.GetTypeat line 38, which is available through theopen Systemimport already present. The SuppressMessage target correctly updated from@294to@292, matching the actual line where thetransformfunction is defined.However, line-number-based compiler-generated identifiers in SuppressMessage attributes remain fragile and require manual updates whenever code earlier in the file changes. Consider whether a more stable targeting mechanism is available.
Samples/Sample26/Sample26.fsproj (1)
4-4: I need to verify AltCover.Api 8.4.840 compatibility with .NET 10.0. The shell script didn't return the nuspec metadata. Let me search for package information:TargetFramework update to net10.0 is compatible and verified.
AltCover.Api 8.4.840 targets .NET Standard 2.0, which is compatible with .NET 10.0 and higher. The framework update syntax is correct and aligns with the PR objectives. Verified that .NET 10.0 was released as GA/LTS on November 12, 2025 with support through November 10, 2028.
AltCover.Fake/AltCover.Fake.fsproj (2)
25-26: Central package version management is properly configured.Both
FAKE.Core.TraceandFAKE.DotNet.Clihave explicit versions (6.1.4) defined inDirectory.Packages.props, andManagePackageVersionsCentrallyis enabled inDirectory.Build.props. The removal of version attributes from the PackageReferences is correct.
8-8: Code changes approved - central package management properly configured with no security vulnerabilities.Verification confirms:
- FAKE packages (v6.1.4) have no security vulnerabilities in GitHub advisories
- Central Package Management via Directory.Packages.props is properly configured
- Removing NU1902–NU1904 suppressions safely improves security posture without revealing actual issues
- VersionOverride removal aligns with CPM pattern
Samples/Sample27/Sample27.fsproj (1)
4-4: Framework update to .NET 10.0 looks good — verify build compatibility before merging.The change from
net9.0tonet10.0is correct and timely (.NET 10.0 GA was released today, November 12, 2025 as an LTS release). The dual-targeting withnet472is preserved. However, since the project uses implicit package versions, verify in your build environment that:
- Microsoft.NET.Test.Sdk, xunit, xunit.runner.visualstudio, and FSharp.Core resolve to versions compatible with .NET 10.0
- The build succeeds for both
net472andnet10.0target frameworksBuild/Build.fsproj (1)
1-54: No action required—TargetFramework is properly configured.The
TargetFrameworkfor the Build project is correctly set tonet10.0inBuild/Directory.Build.props(line 5). The Build.fsproj project inherits this through the standard SDK auto-import mechanism, so no additional changes are needed.AltCover.Fake.DotNet.Testing.AltCover/AltCover.Fake.DotNet.Testing.AltCover.fsproj (2)
43-45: FAKE package version upgrade from 6.0.0 to 6.1.4—verify compatibility.Central package management is properly configured via Directory.Packages.props with
ManagePackageVersionsCentrallyenabled. However, removing theVersionOverride="6.0.0"attributes results in upgrading FAKE packages from 6.0.0 to 6.1.4. Verify that this patch version bump is compatible with the rest of the codebase:
- FAKE.Core.Environment: 6.0.0 → 6.1.4
- FAKE.Core.Process: 6.0.0 → 6.1.4
- FAKE.DotNet.Cli: 6.0.0 → 6.1.4
8-8: Good cleanup: Security and downgrade warnings are now visible.Removing the suppression of NU1605 (package downgrade) and NU1902/NU1903/NU1904 (security vulnerabilities) is correct. Verification shows no security vulnerabilities are present in the FAKE packages, and the project already uses central package management via
Directory.Packages.propswith versions pinned to 6.1.4. The removal of inlineVersionOverride="6.0.0"aligns properly with this centralized approach.AltCover.Recorder2.Tests/AltCover.Recorder2.Tests.fsproj (1)
4-4: Code change verified and approved..NET 10.0 is officially released as GA with LTS support through November 10, 2028, aligning perfectly with this PR's objectives. The framework upgrade on line 4 is confirmed correct. Package compatibility verification completed without warnings, and the removal of NU1702 suppressions is now reflected in the final project state. All referenced packages (NUnit 3.14.0, Microsoft.NET.Test.Sdk ecosystem) are compatible with net10.0.
AltCover.Visualizer/Persistence.fs (3)
52-52: LGTM! Explicit sequence expression for .NET 10.0 compatibility.The explicit
seqkeyword clarifies the type and is likely required for F# compiler compatibility with .NET 10.0. The change maintains the existing behavior.
144-144: LGTM! Consistent with the non-NET472 path.The explicit
seqkeyword is applied consistently to the NET472 code path, maintaining parity with the change at line 52.
52-62: Pre-existing range bounds issue warrants developer awareness.The code at lines 52 and 144 uses
seq { 0 .. NMonitors }, which based on GTK# API documentation, if NMonitors returns a count N, valid monitor indices should be 0..N-1. This creates a potential off-by-one where indexNMonitorswould be invalid.However, the code appears to be working in production, suggesting the GTK# binding may handle out-of-range indices gracefully, or the filtering/fallback logic masks any issues. Since this is pre-existing code not changed by this PR, it's not a blocker, but the developer should be aware for potential future refactoring: consider using
seq { 0 .. display.NMonitors - 1 }to align with documented GTK# behavior.AltCover.Avalonia/AltCover.Avalonia.fsproj (4)
13-13: Verify the warning suppression change.The warning suppression was changed from NU190x (package security advisories) to NETSDK1206 (transitive package vulnerabilities). This change narrows the scope of suppressed warnings. Ensure this is intentional and that any previously suppressed NU190x warnings have been addressed or are no longer applicable.
16-21: LGTM: Consistent TRACE definition across configurations.The consolidation of DefineConstants to consistently include TRACE in both Debug and Release configurations is appropriate and enables diagnostic tracing in production builds when needed.
23-66: LGTM: ItemGroup formatting improvements.The restructuring and formatting of ItemGroup sections improves readability without changing semantics. All dependencies, resources, and project references are appropriately configured.
5-5: Verify whether AltCover.Avalonia projects should be updated to net10.0.The PR targets .NET 10.0 support, but both Avalonia projects remain on net8.0:
- AltCover.Avalonia.fsproj: Uses Avalonia 0.10.18 (2021), which is incompatible with net10.0. Updating the target framework would require upgrading Avalonia first.
- AltCover.Avalonia11.fsproj: Uses Avalonia 11 (defined by
AVALONIA11constant), which supports net10.0. This project could potentially target net10.0 but currently does not.Confirm whether:
- The legacy 0.10.18 project is intentionally maintained on net8.0 and should not be updated as part of this release.
- The Avalonia11 project should be updated to net10.0 to align with the PR's net10.0 support objective.
Demo/MultiTest/Test2/Test2.fsproj (1)
4-4: Framework upgrade to .NET 10.0 is confirmed compatible.The update from
net9.0tonet10.0aligns with the PR objectives. Package compatibility is confirmed via NuGet metadata:
Microsoft.NET.Test.Sdkv17.9.0 is listed as compatible with net10.0FSharp.Corev8.0.200 runs on .NET 10 via .NET Standard compatibilityOptional: For explicit .NET 10 support, consider upgrading to newer FSharp.Core releases (10.x) which include dedicated net10.0 targets.
global.json (1)
3-3: LGTM! SDK version updated for .NET 10.0.The SDK version bump to 10.0.100 aligns with the PR objective to add .NET 10.0 support.
RegressionTesting/SoakTest1/SoakTest1.fsproj (1)
5-5: LGTM! Target framework updated to .NET 10.0.The framework upgrade is consistent with the PR-wide .NET 10.0 adoption.
Build/Directory.Build.props (1)
5-5: LGTM! Build configuration updated for .NET 10.0.This directory-level target framework update will apply to multiple build projects, ensuring consistent .NET 10.0 support across the solution.
Samples/Sample14/Sample14/Sample14.csproj (1)
5-5: LGTM! Sample project updated to .NET 10.0.Samples/Sample10/Sample10.csproj (1)
4-4: LGTM! Test project updated to .NET 10.0.AltCover.Engine/AltCover.Engine.fsproj (2)
8-9: NoWarn suppression added for framework upgrade.The NU1702 warning suppression is appropriate during framework upgrades, as it handles package/project dependency mismatches across different target frameworks.
78-113: System.IO.Compression removal requires manual verification—critical compiler risk detected.Code in
AltCover.Engine/CommandLine.fsusesZipFile.Open(), which typically requires theSystem.IO.Compression.ZipFileNuGet package (distinct from the built-inSystem.IO.Compressionnamespace). WhileGZipStream,DeflateStream, andZipArchiveare built-in to netstandard2.0,ZipFilestatic methods may not be available without the explicit package reference.Before merging, verify that the full
.fsprojfile still includes appropriate package references forSystem.IO.Compression.ZipFileifZipFile.Open()calls are used, or confirm the code paths are restructured to avoid them.Samples/Sample32/Sample32.csproj (1)
5-5: LGTM! Multi-target frameworks updated to include .NET 10.0.The project correctly maintains .NET Framework 4.7.2 support while upgrading to .NET 10.0.
Samples/Sample7/Sample7.fsproj (1)
4-4: LGTM! F# sample project updated to .NET 10.0.OutputBuildProps.props (1)
41-41: LGTM: Warning suppression cleanup.Removing FS0203 (F# indentation warning) from the non-Debug configuration aligns with the broader warning-suppression consolidation observed across the PR.
AltCover.Api.Tests/AltCover.Api.Tests.fsproj (1)
4-4: LGTM: Framework upgrade to .NET 10.0.The TargetFrameworks update from net9.0;net472 to net10.0;net472 is consistent with the PR's objective to support .NET 10.0.
.gitignore (1)
92-92: LGTM: Updated ignore pattern for new dependency version.Adding BlackFox.CommandLine.9.dll to the ignore list is consistent with the existing pattern for version 8, likely reflecting a dependency upgrade for .NET 10.0 support.
AltCover.TestData/AltCover.Tests.xml (1)
4-4: LGTM: Framework upgrade to .NET 10.0.The TargetFrameworks update from net9.0;net472 to net10.0;net472 is consistent with the PR's objective.
AltCover.Monitor.Tests/AltCover.Monitor.Tests.fsproj (1)
4-4: LGTM: Framework upgrade to .NET 10.0.The TargetFrameworks update from net9.0;net472 to net10.0;net472 aligns with the systematic framework upgrade across test projects.
AltCover.Recorder/AltCover.Recorder.csproj (1)
11-11: Verify that suppressing CS1591 is intentional.Adding CS1591 (missing XML documentation) to the global NoWarn list suppresses documentation warnings project-wide. Ensure this aligns with the project's documentation standards and that the suppression is intentional rather than hiding documentation gaps that should be addressed.
Demo/Echo/Echo/Echo.csproj (1)
5-5: LGTM: Framework upgrade to .NET 10.0.The TargetFramework update from net9.0 to net10.0 is consistent with the PR's objective to support .NET 10.0.
Samples/Sample2/Sample2.fsproj (1)
4-4: LGTM: Framework upgrade to .NET 10.0.The TargetFrameworks update from net9.0;net472 to net10.0;net472 is consistent with the systematic framework upgrade across sample projects in this PR.
Samples/Sample8/Sample8.csproj (1)
5-5: LGTM! Target framework updated to .NET 10.0.The TargetFrameworks update from net9.0;net20 to net10.0;net20 is consistent with the PR's objective to support .NET 10.0.
AltCover.ValidateGendarmeEmulation/AltCover.ValidateGendarmeEmulation.fsproj (1)
4-4: LGTM! Target frameworks updated to .NET 10.0.The update from net9.0;net472 to net10.0;net472 aligns with the broader .NET 10.0 migration across the repository.
AltCover.Engine.Tests/Program.fs (1)
865-873: LGTM! More idiomatic sequence expression.The change from
{ 0..31 }(list literal) toseq { 0..31 }(sequence expression) makes the code more idiomatic when the result is immediately piped toSeq.map. Both versions produce the same runtime behavior—32 test cases—but the explicitseqis clearer about intent.Samples/Sample4/Sample4.fsproj (1)
5-5: LGTM! Target frameworks updated to .NET 10.0.The TargetFrameworks update is consistent with the repository-wide .NET 10.0 migration.
Demo/Echo/Test/Test.csproj (1)
4-4: LGTM! Target framework updated to .NET 10.0.The update from net9.0 to net10.0 aligns with the PR's migration objective.
Build/DriveApi.fsproj (1)
3-6: LGTM! Framework updated and warning suppressions removed.The project has been updated to target net10.0, and the removal of NoWarn/MSBuildWarningsAsMessages groups suggests warnings have been addressed or are now acceptable for this .NET version.
AltCover.Engine.Tests/Locations.fs (1)
14-43: LGTM! Test paths updated to match .NET 10.0 build outputs.All non-NET472 paths have been correctly updated from net9.0 to net10.0 to match the new build output directory structure. The NET472 paths remain unchanged, which is correct.
Samples/Sample13/Sample13.fsproj (1)
4-4: LGTM! Target framework updated to .NET 10.0.The TargetFramework update is consistent with the repository-wide .NET 10.0 migration.
Samples/Sample17/Sample17.fsproj (1)
4-4: LGTM! Framework target updated correctly.The TargetFrameworks update from net9.0 to net10.0 is clean and consistent with the PR's objective.
Samples/Sample12/Sample12.csproj (1)
5-5: LGTM! Framework target updated correctly.The TargetFrameworks update from net9.0 to net10.0 aligns with the broader upgrade strategy.
AltCover.Recorder.Tests/AltCover.Recorder.Tests.fsproj (2)
4-4: LGTM! Framework target updated correctly.The TargetFrameworks update from net9.0 to net10.0 is consistent with the project-wide upgrade.
9-10: NU1702 suppression appears intentional but undocumented.Based on codebase analysis, the NU1702 suppressions are pattern-based and framework-aware:
- AltCover.Recorder.Tests (net10.0;net472): Conditional suppression for non-net472 targets only, indicating the warning manifests specifically with modern frameworks.
- AltCover.Engine (netstandard2.0): Unconditional suppression.
No git history, code comments, or documentation explain the rationale. However, the project uses centralized package management with explicit versions (NUnit 4.4.0, NUnit3TestAdapter 5.2.0, Microsoft.NET.Test.Sdk 18.0.1, etc.) and demonstrates intentional dependency management elsewhere (e.g., VersionOverride for specific cases).
The pattern suggests this is a legitimate resolution for framework-specific dependency behavior rather than masking critical issues. However, without running a full NuGet dependency analysis or understanding the specific version conflicts triggering NU1702, the underlying reason remains undocumented. Consider adding a comment to the .fsproj explaining the suppression's purpose (e.g., if it's a known transitive dependency downgrade that doesn't affect functionality).
Directory.Build.props (1)
9-9: LGTM! Appropriate for .NET 10.0 upgrade.The
AllowMissingPrunePackageDataproperty suppresses warnings about missing package trimming metadata, which is appropriate during a major framework upgrade when not all dependencies may have updated their trimming annotations yet.Build/msbuildtest.proj (1)
11-12: LGTM! Test infrastructure paths updated consistently.All references to net9.0 paths and build targets have been correctly updated to net10.0, ensuring the MSBuild test infrastructure aligns with the framework upgrade.
Also applies to: 15-15, 25-25, 27-27
FakeForAltCoverBuild.slnx (1)
8-8: LGTM! Solution structure updated.Adding the newly introduced Directory.Build.targets file to the solution items is appropriate for maintaining visibility of build configuration files.
Directory.Build.targets (1)
3-6: Correct severity levels and recommend advisory-specific suppressions instead of blanket NoWarn.The review contains two factual corrections needed:
- NU1903 is high severity, not moderate
- NU1904 is critical severity
Additionally, the recommended approach is to use NuGetAuditSuppress with specific advisory URLs rather than blanket suppression with NoWarn. The current
<NoWarn>approach suppresses all instances of these warning codes; NuGetAuditSuppress allows permanent suppression of specific advisories while still catching new vulnerabilities.To properly address the review's three verification points, the actual NuGet restore output should be inspected to identify:
- Which specific package(s) trigger NU1903/NU1904 and their associated advisory URLs
- Whether those packages are declared as
PrivateAssets="all"orExcludeAssetsto confirm they're build-only- The details of each advisory to confirm acceptability
.config/dotnet-tools.json (1)
6-6: All specified tools are confirmed compatible with .NET 10.0.dotnet-fsharplint 0.26.4, nbgv 3.9.50, and dotnet-reportgenerator-globaltool 5.5.0 all target net8.0 with computed compatibility through net10.0 per NuGet.
Samples/Sample23/Sample23.csproj (1)
4-4: LGTM! Framework migration to .NET 10.0.The target framework update from net9.0 to net10.0 is consistent with the PR objectives and properly maintains multi-targeting with net472.
Samples/Sample21/Sample21.csproj (2)
2-2: LGTM! Documentation updated for .NET 10.0.The inline test command comment has been properly updated to reference net10.0.
5-5: LGTM! Framework migration to .NET 10.0.The target framework update from net9.0;net472 to net10.0;net472 is consistent with the PR objectives.
AltCover.Engine/ProgramDatabase.fs (1)
124-136: LGTM! Sequence expression refactoring.The change from a list literal
{ 1..streams }to an explicit sequence expressionseq { 1..streams }affects evaluation strategy (eager to lazy), but since the result is immediately consumed viaSeq.mapandSeq.toArray, the functional behavior remains equivalent. This aligns with similar refactorings across the codebase in this PR.AltCover.Engine/NativeJson.fs (1)
442-445: LGTM! Sequence expression refactoring.The change from
{ 0..14 }toseq { 0..14 }is consistent with similar refactorings throughout this PR. Since the result is immediately converted to a Map, there's no functional difference.AltCover.Async/AltCover.Async.csproj (1)
9-10: LGTM! Warning suppression consolidation.Moving CS1591 (missing XML comments) suppression from per-configuration to the global PropertyGroup is a cleaner approach and aligns with the PR's pattern of consolidating warning suppressions across the codebase.
Samples/Sample4/Sample4LongForm.fsproj (1)
66-67: LGTM! Intermediate paths updated for .NET 10.0.The Compile Remove paths have been correctly updated from net9.0 to net10.0 to match the framework migration, ensuring intermediate build artifacts are properly excluded.
AltCover.UICommon/CoverageFile.fs (1)
115-126: LGTM! Sequence expression refactoring.The change from
{ 1..second }toseq { 1..second }is consistent with similar refactorings throughout this PR. Since the result is immediately consumed bySeq.iteri, the functional behavior is preserved.AltCover.Visualizer/AltCover.Visualizer3.fsproj (1)
3-14: NoWarn/MSBuildWarningsAsMessages removed; Visualizer3 still on net8.0 and PackAsTool=true.
- Please confirm warnings don’t now fail the build for this project after suppression removal.
- If this legacy GTK3 tool isn’t meant to be published anymore, consider IsPackable=false or a distinct ToolCommandName to avoid any clash with the Avalonia tool packaging.
AltCover.TestData/OpenCoverForPester.coverlet.xml (1)
160-175: Re‑baseline of sequence/branch points for AsBar()Offsets updated (ordinals 3–7 and related branches). Looks consistent with net10.0 builds. Please verify these align with the freshly generated coverage from the updated pipeline.
AltCover.TestData/AltCoverFSharpTypes.n3.xml (1)
82-90: Sequence point offsets updated for as_bar()Offsets only; counts and ordering preserved. Looks good for net10.0. Please confirm CI artifacts match.
Build/targets.fs (3)
397-406: Framework targeting moved to net10.0 across test helpersSetting Framework=Some "net10.0" in default and coverlet test options looks correct.
Also applies to: 422-424
5111-5163: Rename verification complete—all references updated consistentlyConfirmed no lingering references to
cake5plusTestexist in the repository. The rename tocake6plusTestis properly implemented across all three locations: function definition (line 5111), target wiring (line 7395), and dependency chain (line 7638).
1355-1362: Verify Cake 6.0.0 package structure for FxCop dependency pathsCake 6.0.0 ships with support for .NET 10, 9, and 8, with recommended addin TFMs being net10.0, net9.0, and net8.0. The FxCop setup at lines 1357 and 1361 hard-codes only
lib/net8.0for cake.common and cake.core.Verify whether the Cake.Common and Cake.Core 6.0.0 packages actually include binaries for net9.0 and net10.0. If they do, the dependency directories should include those paths to ensure complete resolution during analysis.
Directory.Packages.props (1)
16-17: Cake 6.0.0 upgrade: validate dependent path assumptionsGiven Build/targets.fs probes Cake assemblies under lib/net8.0 for FxCop deps, please confirm Cake 6 still publishes that TFM (or update those paths accordingly).
AltCover.Engine.Tests/Main.fs (1)
3815-3815: Change is correct and approved.The Sample4 project targets
net10.0(per Sample4.fsproj:<TargetFrameworks>net10.0;net472</TargetFrameworks>), so the test path update tonet10.0is valid and necessary. The binaries will exist at the specified location after the project is built during the test run setup.AltCover.Engine.Tests/AltCover.Engine.Tests.fsproj (1)
4-4: TFM update to net10.0 is consistent with the suiteLooks good; ensure CI agents use an SDK that understands net10.0.
AltCover.Engine.Tests/Visitor.fs (1)
723-723: LGTM - Consistent .NET 10.0 test path updates.All test path and resource name updates are consistent with the .NET 10.0 migration. The changes correctly update assembly paths, embedded resource names, and assertion messages throughout the test suite.
Also applies to: 2837-2837, 2866-2866, 3142-3142, 4821-4823
AltCover.TestData/Sample4-10.native.json (1)
1-383: LGTM - Valid test data file for .NET 10.0.This new test data file contains properly structured native JSON coverage metadata for Sample4.dll targeting .NET 10.0. The structure is consistent with coverage reporting formats, and the file is correctly referenced in the test suite.
AltCover.Visualizer.Tests/AltCover.Visualizer.Tests.fsproj (1)
4-4: LGTM - Project file updated for .NET 10.0.The target framework update from net9.0 to net10.0 is correct, and the removal of the NoWarn 988 suppression suggests this warning may no longer be relevant for .NET 10.0. The changes align with other project file updates in this PR.
Also applies to: 17-18
README.md (1)
77-77: LGTM - Documentation updated for .NET 10.0.The documentation updates correctly reflect the migration to .NET 10.0, including test framework versions, runtime requirements, and build targets. The changes are clear and consistent with the technical updates in the codebase.
Also applies to: 79-79, 85-85, 112-112
Build/merge-coverage.ps1 (1)
4-10: LGTM - Coverage merge inputs updated for .NET 10.0.All OpenCover artifact paths have been correctly updated from net9.0 to net10.0, maintaining consistency with the test execution changes. The unchanged entries (Recorder2Test and Pester) are appropriate as they don't appear to have framework-specific variants.
Summary by CodeRabbit
New Features
Updates
Bug Fixes
Documentation