Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect warnings as errors warnings as messages and warnings not as errors #11342

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

SimaTian
Copy link
Member

Fixes #10871

Only a partial fix due to the TreatWarningsAsErrors issues, for more detail see the linked ticket.

Context

The MSBuild currently has an odd way of handling the WarningsAsErrors, WarningsNotAsErrors and WarningsAsMessages properties as opposed to their MSBuild<> counterparts. This is due to the way that these are translated in a MSBuild target, which has some issues. See the linked tickets.
This is a PR to unify the behavior, with the caveat that the TreatWarningsAsErrors which is also an offender is excluded since it is currently a breaking change for many builds.

Hidden behind changewave 17.14

Changes Made

Unification of
WarningsAsErrors
WarningsNotAsErrors
WarningsAsMessages
with their MSBuild<> counterpart on the engine level.

Testing

Unit testing in WarningsAsMessagesAndErrors_Tests.cs

Notes

I've left the tests for TreatWarningsAsErrors tests in a disabled / commented state. If we want to push forward with the further unification down the line, this will be useful. If this is too far in the future, I can remove them altogether.

@@ -9,6 +9,7 @@
using Shouldly;
using Xunit;
using Xunit.Abstractions;
using static System.Net.WebRequestMethods;
Copy link
Member

Choose a reason for hiding this comment

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

Feels unexpected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Common.targets doesn't promote TreatWarningsAsErrors to MSBuildTreatWarningsAsErrors
3 participants