Skip to content

Conversation

baronfel
Copy link
Member

Part of dotnet/msbuild#12081.

We've trained users to reach for $(NoWarn) as the solution to managing all kinds of warnings - but it doesn't work for Messages. Since that's a good chunk of work on the MSBuild side, let's do something in the near-term for SDK users specifically.

@baronfel baronfel requested review from Copilot and a team June 29, 2025 15:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR lets SDK users suppress the .NET Core SDK preview warning by including its code (NETSDK1057) in the NoWarn property.

  • Introduces a new SuppressNETCoreSdkPreviewMessage property driven by $(NoWarn).Contains('NETSDK1057')
  • Modifies the preview-check target to skip warning emission when suppression is enabled

@@ -329,6 +329,16 @@ Copyright (c) .NET Foundation. All rights reserved.

</Target>

<PropertyGroup>
Copy link
Preview

Copilot AI Jun 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new behavior to suppress the preview message based on NoWarn but lacks corresponding tests to verify this logic. Please add unit or integration tests covering scenarios where NETSDK1057 is and isn’t included in $(NoWarn).

Copilot generated this review using guidance from repository custom instructions.

@baronfel baronfel force-pushed the handle-nowarn-for-sdk-preview-message branch from 8e3d081 to 9b86ba2 Compare June 29, 2025 19:01
Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot thinks you should add a test for the behavior, and it looks like that would be pretty simple to do in GivenThatWeWantAMessageWhenBuildingWithAPreviewSdk.

@baronfel
Copy link
Member Author

Tests added!

@baronfel baronfel enabled auto-merge (squash) June 30, 2025 18:25
@baronfel baronfel merged commit 36a3998 into dotnet:main Jun 30, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants