Add HasNoWarnNU1701 merge logic in project discovery#15090
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end tracking of projects that suppress NuGet warning NU1701 during discovery, ensures this signal is preserved when merging discovery results, and surfaces it via logger warnings. This helps operators understand when compatibility checks may be less reliable due to NU1701 being explicitly suppressed.
Changes:
- Detect
NoWarncontainingNU1701during SDK project discovery and persist it onProjectDiscoveryResult. - Merge
HasNoWarnNU1701acrossProjectDiscoveryResultinstances using logical OR inDiscoveryWorker.MergeProjectDiscovery. - Emit warnings in
ILogger.ReportDiscoveryfor affected projects and add/adjust tests for merge + logging behavior.
Show a summary per file
| File | Description |
|---|---|
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/ILogger.cs | Logs WARN entries for projects where discovery flagged HasNoWarnNU1701. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs | Computes HasNoWarnNU1701 from evaluated NoWarn MSBuild property. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/ProjectDiscoveryResult.cs | Adds HasNoWarnNU1701 field to the discovery model. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/DiscoveryWorker.cs | Merges HasNoWarnNU1701 via OR when combining project discovery results. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/LoggerTests.cs | Adds a test validating NU1701 warning logging and ordering. |
| nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.cs | Updates merge test to verify HasNoWarnNU1701 OR merge behavior. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 2
brettfo
force-pushed
the
brettfo/merge-nowarn-nu1701
branch
from
01672a8 to
40e7abd
Compare
sebasgomez238
approved these changes
May 21, 2026
Merge the HasNoWarnNU1701 property using a simple OR when combining ProjectDiscoveryResult instances. Updated the existing merge test to verify the behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
brettfo
force-pushed
the
brettfo/merge-nowarn-nu1701
branch
from
40e7abd to
84354aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge the
HasNoWarnNU1701property using a simple OR when combiningProjectDiscoveryResultinstances inDiscoveryWorker.MergeProjectDiscovery. Updated the existing merge test to verify the behavior (one result hasfalse, the other hastrue, merged result istrue).