Suppress advertising manifest messages unless verbosity is diagnostic#51560
Suppress advertising manifest messages unless verbosity is diagnostic#51560Copilot wants to merge 9 commits intorelease/10.0.4xxfrom
Conversation
|
@copilot on unsigned .NET versions, we also have a message we include for each manifest which is more than we probably want. I think it's still worth saying it once. Can you reduce the number of times this message is sent to be only once? |
|
Testing done: The message only shows up in manifest mode. Confirmed that with this change, I no longer see it but if I do -v:diag, I will still get the messages as expected. |
Fixed in commit b564f6b. Changed the |
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Changed _validationMessagesDisplayed flag from instance-level to static to ensure "Skipping NuGet package signature verification." message appears only once across all NuGetPackageDownloader instances during workload operations, rather than once per instance. Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
b564f6b to
8e3eb07
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses the issue of advertising manifest messages being displayed during workload install operations by ensuring they only appear when verbosity is set to diagnostic. Additionally, it fixes duplicate "Skipping NuGet package signature verification" messages by making the display flag static.
Changes:
- Added
IsDiagnostic()extension method to check for diagnostic verbosity levels - Modified WorkloadManifestUpdater to guard advertising manifest error messages with the
_displayManifestUpdatesflag - Updated WorkloadInstallCommand, WorkloadUpdateCommand, and WorkloadListCommand to use
IsDiagnostic()instead ofIsDetailedOrDiagnostic()when creating WorkloadManifestUpdater instances - Made
_validationMessagesDisplayedflag static in NuGetPackageDownloader to prevent duplicate signature verification messages - Added new test to verify message suppression and updated existing tests to explicitly set the
displayManifestUpdatesparameter
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/Extensions/CommonOptionsExtensions.cs | Added IsDiagnostic() extension method for VerbosityOptions |
| src/Cli/dotnet/Commands/Workload/Install/WorkloadManifestUpdater.cs | Added conditional checks around error messages using _displayManifestUpdates flag |
| src/Cli/dotnet/Commands/Workload/Install/WorkloadInstallCommand.cs | Changed from IsDetailedOrDiagnostic() to IsDiagnostic() for displayManifestUpdates parameter |
| src/Cli/dotnet/Commands/Workload/Update/WorkloadUpdateCommand.cs | Changed from IsDetailedOrDiagnostic() to IsDiagnostic() for displayManifestUpdates parameter |
| src/Cli/dotnet/Commands/Workload/List/WorkloadListCommand.cs | Changed to IsDiagnostic() for displayManifestUpdates parameter |
| src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs | Changed _validationMessagesDisplayed from instance to static field |
| test/dotnet.Tests/CommandTests/Workload/Install/GivenWorkloadManifestUpdater.cs | Updated existing tests with explicit displayManifestUpdates parameter and added new suppression test |
| test/dotnet.Tests/CommandTests/Workload/Install/GivenDotnetWorkloadInstall.cs | Updated test to move detailed verbosity to hide test and rename test method |
| eng/Versions.props | Incremented VersionSDKMinor from 2 to 3 |
…ix-advertising-manifest-issues
d98bded to
b20d157
Compare
Summary: Suppress Advertising Manifest Messages
This PR fixes the issue where advertising manifest messages are displayed during workload install operations. According to the requirements, these messages should only appear when verbosity is set to diagnostic (diag).
What was the problem?
Users were seeing these messages during normal workload install operations:
These messages were being displayed even at normal verbosity levels, which cluttered the output.
Additionally, on unsigned .NET versions, the message "Skipping NuGet package signature verification." was appearing multiple times (once per NuGetPackageDownloader instance).
What changed?
IsDiagnostic()extension method - Returns true only fordiagnosticordiagverbosity levels_displayManifestUpdatesflagIsDiagnostic()instead ofIsDetailedOrDiagnostic()when creating WorkloadManifestUpdater instances_validationMessagesDisplayedflag to static in NuGetPackageDownloader to ensure the message appears only once globallyVerification:
✅ All 23 WorkloadManifestUpdater tests pass
✅ All 43 WorkloadInstall tests pass (1 pre-existing failure unrelated to this change)
✅ Code review completed with all feedback addressed
✅ CodeQL security scan passed
✅ Build succeeds
Files Modified:
src/Cli/dotnet/Extensions/CommonOptionsExtensions.cs- Added IsDiagnostic() methodsrc/Cli/dotnet/Commands/Workload/Install/WorkloadManifestUpdater.cs- Guarded error messagessrc/Cli/dotnet/Commands/Workload/Install/WorkloadInstallCommand.cs- Changed to IsDiagnostic()src/Cli/dotnet/Commands/Workload/Update/WorkloadUpdateCommand.cs- Changed to IsDiagnostic()src/Cli/dotnet/Commands/Workload/List/WorkloadListCommand.cs- Changed to IsDiagnostic()src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs- Made validation message flag statictest/dotnet.Tests/CommandTests/Workload/Install/GivenWorkloadManifestUpdater.cs- Updated teststest/dotnet.Tests/CommandTests/Workload/Install/GivenDotnetWorkloadInstall.cs- Updated testsOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.