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

Make Analyzer crashes a warning #90358

Merged
merged 3 commits into from
Aug 11, 2023
Merged

Make Analyzer crashes a warning #90358

merged 3 commits into from
Aug 11, 2023

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Aug 11, 2023

Fixes #78412

@stephentoub - you mentioned:

warnings fail the build and there's nothing a developer can do to address them short of disabling the rule (if any are non-deterministic). We can try it once we're clean, though, and see how it goes.

If new warnings are introduced we can add a NoWarn to the project and file an issue. Agreed that non-deterministic warnings will require undoing this. Hopefully we can track such a thing against an issue and undo it with fixed.

To get clean I suppressed this warning and filled the following issues:
#90356
#90357

@ghost
Copy link

ghost commented Aug 11, 2023

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #78412

@stephentoub - you mentioned:

warnings fail the build and there's nothing a developer can do to address them short of disabling the rule (if any are non-deterministic). We can try it once we're clean, though, and see how it goes.

If new warnings are introduced we can add a NoWarn to the project and file an issue. Agreed that non-deterministic warnings will require undoing this. Hopefully we can track such a thing against an issue and undo it with fixed.

To get clean I suppressed this warning and filled the following issues:
#90356
#90357

Author: ericstj
Assignees: -
Labels:

area-Meta

Milestone: -

@stephentoub
Copy link
Member

To get clean I suppressed this warning and filled the following issues:

I don't see any suppressions. Did you maybe miss a commit?

@ericstj
Copy link
Member Author

ericstj commented Aug 11, 2023

To get clean I suppressed this warning and filled the following issues:

I don't see any suppressions. Did you maybe miss a commit?

Right you are 🤦‍♂️I waited to build clean and filed all the issues but forgot to commit the suppressions.

@stephentoub
Copy link
Member

Looks like there are more failures.

@ericstj
Copy link
Member Author

ericstj commented Aug 11, 2023

Looks like there are more failures.

Yeah I figured. I got a clean Windows Libs build w/ CoreCLR but other flavors need suppressions.

@ericstj ericstj merged commit 7bc9f6b into dotnet:main Aug 11, 2023
169 of 171 checks passed
@@ -46,7 +46,8 @@
<!-- Override InformationalVersion during servicing as it's returned via public api. -->
<InformationalVersion Condition="'$(PreReleaseVersionLabel)' == 'servicing'">$(ProductVersion)</InformationalVersion>
<InformationalVersion Condition="'$(StabilizePackageVersion)' == 'true'">$(ProductVersion)</InformationalVersion>
<NoWarn>$(NoWarn),0419,0649,CA2249,CA1830</NoWarn>
<!-- AD0001 : https://github.com/dotnet/runtime/issues/90356 -->
<NoWarn>$(NoWarn),0419,0649,CA2249,CA1830;AD0001</NoWarn>
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered it but the generator was referenced in each project and it made more sense to put this in the same spot the generator was referenced.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn on analyzer crashes in runtime repo
3 participants