-
Notifications
You must be signed in to change notification settings - Fork 280
Add ImplicitMSTestUsings property to control MSTest implicit usings #6313
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
base: main
Are you sure you want to change the base?
Conversation
b3452ae
to
c44d51b
Compare
The addition of this namespace to Implicit Usings is causing us problems because we make use of the System.ComponentModel.DescriptionAttribute in more places. * Add ImplicitMSTestUsings property to control MSTest implicit usings * Add comprehensive tests for ImplicitMSTestUsings property
I'm quite curious about this. What are you using |
@@ -31,7 +31,7 @@ | |||
Ensure feature is available and user hasn't opted-out from it. | |||
See https://github.com/dotnet/sdk/blob/f9fdf2c7d94bc86dc443e5a9ffecbd1962b1d85d/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.CSharp.props#L26-L34 | |||
--> | |||
<ItemGroup Condition=" '$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable' "> | |||
<ItemGroup Condition="('$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable') And ('$(ImplicitMSTestUsings)' == '' Or '$(ImplicitMSTestUsings)' == 'true' Or '$(ImplicitMSTestUsings)' == 'enable')"> |
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.
I would simply go with DisableMSTestImplicitUsings
and only check whether it's set to true.
<ItemGroup Condition="('$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable') And ('$(ImplicitMSTestUsings)' == '' Or '$(ImplicitMSTestUsings)' == 'true' Or '$(ImplicitMSTestUsings)' == 'enable')"> | |
<ItemGroup Condition="('$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable') And '$(DisableMSTestImplicitUsings)' != 'true'"> |
@@ -25,7 +25,7 @@ | |||
Ensure feature is available and user hasn't opted-out from it. | |||
See https://github.com/dotnet/sdk/blob/f9fdf2c7d94bc86dc443e5a9ffecbd1962b1d85d/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.CSharp.props#L26-L34 | |||
--> | |||
<ItemGroup Condition="'$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable'"> | |||
<ItemGroup Condition="('$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable') And ('$(ImplicitMSTestUsings)' == '' Or '$(ImplicitMSTestUsings)' == 'true' Or '$(ImplicitMSTestUsings)' == 'enable')"> |
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.
Same suggestion applies here
@@ -5,7 +5,7 @@ | |||
Ensure feature is available and user hasn't opted-out from it. | |||
See https://github.com/dotnet/sdk/blob/f9fdf2c7d94bc86dc443e5a9ffecbd1962b1d85d/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.CSharp.props#L26-L34 | |||
--> | |||
<ItemGroup Condition="'$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable'"> | |||
<ItemGroup Condition="('$(ImplicitUsings)' == 'true' Or '$(ImplicitUsings)' == 'enable') And ('$(ImplicitMSTestUsings)' == '' Or '$(ImplicitMSTestUsings)' == 'true' Or '$(ImplicitMSTestUsings)' == 'enable')"> |
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.
Same suggestion applies here
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.
I'm also curious, wouldn't the following be a reasonable workaround for you?
<ItemGroup>
<Using Remove="Microsoft.VisualStudio.TestTools.UnitTesting" />
</ItemGroup>
Maybe we should move our <Using Include="..." />
to be in props
instead of targets
so that it's defined early enough that you can easily remove it later?
The addition of this namespace to Implicit Usings is causing us problems because we make use of the System.ComponentModel.DescriptionAttribute in more places.