Remove warnings from terminal when compiling the test runner. #91
Conversation
nicrowe00
force-pushed
the
warnings
branch
2 times, most recently
from
ff2ff38 to
c51809d
Compare
A fix for the test runner that patches most of the warnings that are reported in the terminal when compiling the test runner. Some warnings were suppressed as it was too time-consuming or there were impacts to other parts of the test runner when they were fixed.
|
CentOS 9 and 10 are unrelated to this PR. |
| public List<string> IgnoredRIDs { get; set; } = new(); | ||
| public List<string> SkipWhen { get; set; } = new(); | ||
| #pragma warning disable CA1051 // Do not declare visible instance fields | ||
| public List<string> IgnoredRIDs = new(); |
There was a problem hiding this comment.
Add a {get; set;} so these become properties instead of instance fields. Then you can omit the warning supressions.
| public class Program | ||
| #pragma warning restore CA1052 // Static holder types should be Static or NotInheritable |
There was a problem hiding this comment.
Why not add the static modifier instead of suppressing these warnings?
| @@ -18,7 +18,9 @@ public List<string> CurrentIds | |||
|
|
|||
| public List<string> ComputePlatformIds(string[] osReleaseLines, string lddVersionOutput) | |||
| { | |||
| #pragma warning disable CA1308 // Normalize strings to uppercase | |||
There was a problem hiding this comment.
I've never had this warning. Do we have some specific analyzer settings?
Anyway, I'd simplify this code to (which probably won't get rid of the warning):
string arch = RuntimeInformation.OSArchitecture.ToString().ToLowerInvariant();Or if you want to do something fancy:
static class EnumExtensionMethods
{
public static string ToLowerInvariant(this Enum e)
=> e.ToString().ToLowerInvariant();
}
| public Task<string> GenerateNuGetConfig(List<string> urls, string nugetConfig = null) | ||
| #pragma warning restore CA1822 // Mark members as static | ||
| { | ||
| if( !urls.Any() && nugetConfig == null ) | ||
| #pragma warning disable CA2208 // Instantiate argument exceptions correctly | ||
| throw new ArgumentNullException(); | ||
| #pragma warning restore CA2208 // Instantiate argument exceptions correctly |
There was a problem hiding this comment.
if (!urls.Any())
ArgumentNullException.ThrowIfNull(nugetConfig);
| @@ -36,7 +36,9 @@ public static IEnumerable<string> LocalProjectCruft() | |||
| yield return "project.lock.json"; | |||
| } | |||
|
|
|||
| public async Task CleanProjectLocalDotNetCruftAsync() | |||
| #pragma warning disable CA1822 // Mark members as static | |||
There was a problem hiding this comment.
Similar to https://github.com/redhat-developer/dotnet-bunny/pull/91/files#r1888234678, I'm not used to seeing this message though I often don't add the static keyword to members that are not using instance members. I wonder if we have some specific analyser settings that trigger this?
|
@nicrowe00 thanks for tacking this! Some of the changes you are making made me wonder if the analyzers you are fixing against are more strict than what I'm used to. I see there are some analyser references being made: dotnet-bunny/Turkey/Turkey.csproj Lines 17 to 19 in 116605a If we want to keep these analyzers but don't want to strictly apply every rule (for example requiring the |
A fix for the test runner that patches most of the warnings that are reported in the terminal when compiling the test runner. Some warnings were suppressed as it was too time-consuming or there were impacts to other parts of the test runner when they were fixed.
Fixes #83