-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove extra usings #7193
Remove extra usings #7193
Conversation
I love removing the ones that are really unused, but the added |
When there's some error in VS, and building fails, I end up scrolling through a long list of warnings about unused using statements before I get to the real problem. It isn't a huge deal to me, but it is a little annoying. |
Thanks for doing this Forgind. It makes actual warnings that need to be addressed more obvious. |
This PR now has conflicts. I am pretty excited for zero warnings. |
#if NETCOREAPP | ||
using System.Linq; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can fix this to just actually remove the need: remove FEATURE_TYPE_GETINTERFACE
since it's available in all targets now.
src/MSBuild.UnitTests/XMake_Tests.cs
Outdated
#if FEATURE_CULTUREINFO_CONSOLE_FALLBACK | ||
using System.Globalization; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is now available everywhere too, so we can delete the feature flag.
Just ran into a couple of these and tried enabling the warning for all files (dotnet/roslyn#41640 (comment)):
This doesn't bother me personally but there's more potential cleanup. |
I'm assuming you ran build.cmd after setting that? Those are presumably all warnings in the 18 projects that aren't part of MSBuild.Dev.slnf, so they don't bother me. |
They are all in the core projects, looks like:
|
Interesting; not sure why they aren't showing up in VS, then. If it starts bothering me, I might make another PR; I'm assuming this wasn't bothering you; it was just something you noticed. |
Per the documentation for the rule and the linked dotnet/roslyn#41640 (comment), setting this property globally will turn on the rule at the build time: <PropertyGroup>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup> |
Fixes #7193 Context Enforce no unused usings in the code Changes Made Removed or conditionalized using statements as reported by the compiler. Set property to true. Testing Local build
Basically just made all the warnings in VS related to unnecessary using statements go away.