allow compilation with vs2026 and cleanup some nuget packages#339
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe PR updates multiple project files: in src/NuGetUtility/NuGetUtility.csproj it replaces NuGet.Commands and NuGet.Packaging with NuGet.ProjectModel, removes the net6.0 Microsoft.Build runtime-excluded ItemGroup, separates target-framework ItemGroups so net9.0 uses Microsoft.Build 17., adds net10.0 with Microsoft.Build 18.0., and updates the net472 Microsoft.Build reference to 18.0.*. Test projects bump TUnit from 1.1.10 to 1.2.11. Two C++ test project files remove per-configuration PlatformToolset entries and add an unconditional empty LegacyPlatformToolset property. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
Comment |
d436441 to
7b7bf63
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/targets/EmptyCppProject/EmptyCppProject.vcxproj (1)
51-53: LegacyPlatformToolset placeholder looks good; optional clarity improvementAdding an unconditional
<LegacyPlatformToolset />keeps the test project free from a hard-codedPlatformToolsetwhile still exposing a property for tooling/tests, which is reasonable. If this is intentionally only for test infrastructure, consider adding a brief XML comment orLabelto make that purpose explicit for future maintainers, but it’s not required.src/NuGetUtility/NuGetUtility.csproj (1)
31-37: Microsoft.Build per‑TFM setup aligns with VS2026 goals; consider pinning net9.0 versionUsing
Microsoft.Build17.* fornet9.0and 18.0.* fornet10.0/net472is consistent with the expected MSBuild major versions for newer VS releases and should help with VS2026 compatibility. One minor nit:net8.0is pinned to17.11.*whilenet9.0floats on17.*; if you care about deterministic restores/builds, you might want to pinnet9.0to a specific 17.x (likely the same asnet8.0, or whatever VS2026 mandates) to avoid surprises as new 17.x versions ship. If you intentionally wantnet9.0to float, maybe add a brief comment to document that choice.Also, please verify that
Microsoft.Build18.0.* onnet472matches the MSBuild version actually installed with VS2026 on your build agents.Also applies to: 39-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/NuGetUtility/NuGetUtility.csproj(1 hunks)tests/NuGetUtility.UrlToLicenseMapping.Test/NuGetUtility.UrlToLicenseMapping.Test.csproj(1 hunks)tests/SPDXLicenseMatcher.Test/SPDXLicenseMatcher.Test.csproj(1 hunks)tests/targets/EmptyCppProject/EmptyCppProject.vcxproj(1 hunks)tests/targets/SimpleCppProject/SimpleCppProject.vcxproj(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/SPDXLicenseMatcher.Test/SPDXLicenseMatcher.Test.csproj
- tests/NuGetUtility.UrlToLicenseMapping.Test/NuGetUtility.UrlToLicenseMapping.Test.csproj
- tests/targets/SimpleCppProject/SimpleCppProject.vcxproj
⏰ 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). (16)
- GitHub Check: test_windows (net10.0)
- GitHub Check: check_licenses_net472 (App)
- GitHub Check: check_licenses_net472 (ProjectWithReferenceContainingFileLicense)
- GitHub Check: check_licenses_net472 (ProjectWithReferenceContainingLicenseExpression)
- GitHub Check: test_windows (net8.0)
- GitHub Check: check_version_command_net472
- GitHub Check: check_licenses_net472 (Tests)
- GitHub Check: test_windows (net9.0)
- GitHub Check: test_windows (net472)
- GitHub Check: test_url_to_license_mapping
- GitHub Check: test (macos-latest, net9.0)
- GitHub Check: test (macos-latest, net10.0)
- GitHub Check: test (macos-latest, net8.0)
- GitHub Check: test_file_license_matching (ubuntu-latest)
- GitHub Check: test_file_license_matching (macos-latest)
- GitHub Check: test_file_license_matching (windows-latest)
🔇 Additional comments (1)
src/NuGetUtility/NuGetUtility.csproj (1)
23-23: NuGet.Packaging dependency removed but still used in code—compilation will failThe
NuGet.Packagingpackage has been removed from the project file, but at least 7 source files still actively import and use types from it:
LicenseMetadata.cs: usesNuGet.Packaging.LicenseType,NuGet.Packaging.LicenseMetadataWrappedPackageMetadata.cs: usesNuGet.PackagingWindowsPackagesConfigReader.cs: usesNuGet.Packaging.PackagesConfigReaderGlobalPackagesFolderUtility.cs: usesNuGet.Packaging.Core.PackageIdentityCachingPackageMetadataResource.cs: usesNuGet.Packaging.Core.PackageIdentityWrappedPackageDownloader.cs: usesNuGet.Packaging.IPackageDownloaderCachingFindPackageByIdResource.cs: usesNuGet.Packaging.Core.PackageIdentity,NuGet.Packaging.IPackageDownloaderEither restore the
NuGet.Packagingpackage reference, or refactor the code to eliminate the dependency.⛔ Skipped due to learnings
Learnt from: sensslen Repo: sensslen/nuget-license PR: 327 File: .github/workflows/release.yml:38-38 Timestamp: 2025-11-07T21:33:27.230Z Learning: In the release workflow (.github/workflows/release.yml), the test assembly pattern `NuGet*.Test.dll` intentionally excludes SPDXLicenseMatcher.Test.dll from the release workflow tests.
7b7bf63 to
362f929
Compare
|



Summary by CodeRabbit
Chores
Tests