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

Stop disabling CA1416 Fixes #6376 #7471

Merged
merged 2 commits into from
Mar 24, 2022

Conversation

Forgind
Copy link
Member

@Forgind Forgind commented Mar 16, 2022

Fixes #6376

Context

We had disabled these before we had the more convenient SupportedOSPlatformGuard. Now we can use that instead.

Changes Made

Replaced #pragma warning disable CA1416s with SupportedOSPlatform("windows")

Testing

Built

@@ -624,9 +624,7 @@ internal static bool IsMaxPathLegacyWindows()
}
}

// CA1416 warns about code that can only run on Windows, but we verified we're running on Windows before this.
// This is the most reasonable way to resolve this part because other ways would require ifdef'ing on NET472.
#pragma warning disable CA1416
Copy link
Member

Choose a reason for hiding this comment

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

There should be a corresponding

#pragma warning restore CA1416

That should also be removed (for all of these).

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Mar 16, 2022
@rainersigwald rainersigwald merged commit 7111186 into dotnet:main Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update OS-detection checks to support analyzers
3 participants