Skip to content

Remove warnings from terminal when compiling the test runner. #91

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicrowe00
Copy link
Contributor

@nicrowe00 nicrowe00 commented Dec 5, 2024

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

@nicrowe00 nicrowe00 force-pushed the warnings branch 3 times, most recently from c51809d to a5cb32c Compare December 5, 2024 12:32
@nicrowe00
Copy link
Contributor Author

CentOS 9 and 10 are unrelated to this PR.

@nicrowe00 nicrowe00 requested review from tmds and omajid December 5, 2024 12:54
@tmds
Copy link
Member

tmds commented Dec 17, 2024

@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:

<PackageReference Include="Microsoft.CodeQuality.Analyzers" Version="2.6" />
<PackageReference Include="Microsoft.NetCore.Analyzers" Version="2.6" />
<PackageReference Include="Text.Analyzers" Version="2.6" />

If we want to keep these analyzers but don't want to strictly apply every rule (for example requiring the static) then we can suppress those at the project level or introduce an editorconfig file.

@nicrowe00
Copy link
Contributor Author

Current CI failures are due to release-version-sane, most OS' don't have the latest .NET version yet.

{
return text.Substring(1, text.Length - 2);
}

return text;
}

#pragma warning disable CA1822 // Mark members as static
internal string GetLddVersion()
Copy link
Member

Choose a reason for hiding this comment

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

Other methods are now static, so I am okay with marking this as static too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting this to static causes an error when executing dotnet test -f net6.0 -c Release --verbosity detailed Turkey.Tests

Error: Turkey.Tests/PlatformIdTest.cs(34,30): error CS0176: Member 'PlatformId.GetLddVersion()' cannot be accessed with an instance reference; qualify it with a type name instead

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix warnings reported when compiling the test runner
3 participants