Skip to content

Change WarningLevel to 9999#48687

Merged
RikkiGibson merged 2 commits intodotnet:masterfrom
RikkiGibson:fix-46462
Oct 23, 2020
Merged

Change WarningLevel to 9999#48687
RikkiGibson merged 2 commits intodotnet:masterfrom
RikkiGibson:fix-46462

Conversation

@RikkiGibson
Copy link
Member

Closes #46462

It turns out that we are just so great that we never hit any of our wave 5 warnings. (I did test adding some code that causes a wave 5 warning to verify that this config change does what we expect.)

Tagging @jmarolf for review.

<PropertyGroup>
<LangVersion>preview</LangVersion>
<WarningLevel>4</WarningLevel>
<WarningLevel>9999</WarningLevel>
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't this supposed to be <AnalysisLevel>latest</AnalysisLevel>?

Copy link
Member

Choose a reason for hiding this comment

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

@alrz From what I understand, AnalysisLevel is related to the shipped analyzers (CAxxxx) in .NET SDK, not the compiler diagnostics. See https://docs.microsoft.com/dotnet/fundamentals/code-analysis/overview#latest-updates

Copy link
Member Author

Choose a reason for hiding this comment

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

perhaps we should adopt AnalysisLevel instead anyway..?

Copy link
Member

Choose a reason for hiding this comment

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

it is about analyzers but from the article it does affect warning level as well.

Copy link
Member

Choose a reason for hiding this comment

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

I couldn't see where in the article it mentions warning waves, but if it does affect it, then definitely go with it. If not, then probably add both.

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend setting this to <AnalysisLevel>preview</AnalysisLevel> only if we also want the additional analyzers to be brought along for the ride. As roslyn has rather complex analyzer consumption requirements (as we both build and consume the same analyzers in our build) I recommend <WarningLevel>9999</WarningLevel>

@RikkiGibson
Copy link
Member Author

We'll wait on this until we can decide whether to adopt AnalysisLevel instead with @jmarolf's help. I was concerned that maybe this would opt us in to the analyzers baked-in to the .NET SDK and be either redundant or undesired (or needs to be controlled via the "-runAnalyzers" flag in our build scripts, etc.)

@RikkiGibson RikkiGibson merged commit c4ac59e into dotnet:master Oct 23, 2020
@ghost ghost added this to the Next milestone Oct 23, 2020
@allisonchou allisonchou modified the milestones: Next, 16.9.P2 Nov 24, 2020
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.

Enable warning wave in Roslyn build

6 participants