-
Notifications
You must be signed in to change notification settings - Fork 181
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
Install NUnit.Analyzers 4.1.0 and follow all suggested autofixes #203
Conversation
<PackageVersion Include="Oracle.ManagedDataAccess.Core" Version="3.21.130" Condition="'$(TargetFramework)' == 'netstandard2.1'" /> | ||
<PackageVersion Include="Oracle.ManagedDataAccess" Version="21.13.0" Condition="'$(TargetFramework)' == 'net462'"/> | ||
<PackageVersion Include="Oracle.ManagedDataAccess" Version="21.13.0" Condition="'$(TargetFramework)' == 'net462'" /> |
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.
There was a missing space; fixed by VS 2022's NuGet Package Manager.
} | ||
|
||
private async ValueTask<int> AddAsync(int a, int b, Thread startingThread, bool expectAsync) | ||
private static async ValueTask<int> AddAsync(int a, int b, Thread startingThread, bool expectAsync) |
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.
VS was showing the warning IDE0009
, so I added this here and below.
@@ -110,7 +110,7 @@ public async Task TestAccessingHandleLostTokenWhileKeepaliveActiveDoesNotBlock() | |||
handle.HandleLostToken.Register(() => { }); | |||
} | |||
}); | |||
Assert.IsTrue(await accessHandleLostTokenTask.TryWaitAsync(TimeSpan.FromSeconds(5))); | |||
Assert.That(await accessHandleLostTokenTask.TryWaitAsync(TimeSpan.FromSeconds(5)), Is.True); |
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.
Drawing attention here.
Assert.That(await accessHandleLostTokenTask.TryWaitAsync(TimeSpan.FromSeconds(5)), Is.True); | |
Assert.That(await accessHandleLostTokenTask.TryWaitAsync(TimeSpan.FromSeconds(5))); |
is also valid and is called the condensed form. For now, I went with the more verbose yet clearer form, but in some cases (e.g. https://github.com/madelson/DistributedLock/pull/203/files#diff-da270224eb19c911add476281ecc4508595b322d7a34c53254e4a82bcaee6e50R24), removing , Is.True
can be more readable.
This PR closes #202.
Steps taken:
NUnit.Analyzers
4.1.0 through VS 2022's built-in NuGet Package Manager.nunit
to the latest version 4.1.0 and noticed that there were some NUnit diagnostics whose severities were not warnings. (I didn't realize there were non-warning diagnostics, but in retrospect, I should've updated.editorconfig
to set the severity to warning or above for allClassicAssert
-related diagnostics in https://docs.nunit.org/articles/nunit-analyzers/NUnit-Analyzers.html. Also, I didn't realize theCodeGen
project hadNUnit
installed, so I installedNUnit.Analzyers
there as well.)