Skip to content
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

Task add style cope and fxcop analyzers #1258

Conversation

WojciechNagorski
Copy link
Contributor

@WojciechNagorski WojciechNagorski commented Oct 1, 2019

In this PR I added StyleCop and FxCop packages for every project in the solution.

In CodingStyle.ruleset file you can find configuration based on dotnet/corefx repo: https://github.com/dotnet/corefx/blob/06007cdedee9cfb096d884016a99e3a64bdccb45/src/Common/src/CoreLib/CodeAnalysis.ruleset
I've removed some line. Please validate this list before merge.

There you can find similar props file from dotnet/corefx: https://github.com/dotnet/corefx/blob/3d37c15b43be2a36de5dc7f9076584407b430175/eng/Analyzers.props

PR for Hacktoberfest 😄

@adamsitnik
Copy link
Member

Wojciech how many warnings do you get when you perform a clean build of the solution with all these rules enabled?

@WojciechNagorski
Copy link
Contributor Author

@adamsitnik

There is a lot of SA1000 https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1000.md
image

If you decide to clean the code, I can make one bulk PR for simple rules in no time.

@adamsitnik
Copy link
Member

@AndreyAkinshin I like the concept of having some static code analysis, are you ok with adding StyleCop and FxCop?

adamsitnik
adamsitnik previously approved these changes Oct 2, 2019
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

LGTM!

@WojciechNagorski
Copy link
Contributor Author

Is there any update on this topic? Should I improve something?

@adamsitnik
Copy link
Member

@AndreyAkinshin ping

1 similar comment
@WojciechNagorski
Copy link
Contributor Author

@AndreyAkinshin ping

@AndreyAkinshin AndreyAkinshin merged commit 4740478 into dotnet:master Mar 2, 2020
@AndreyAkinshin
Copy link
Member

@WojciechNagorski thanks!

@AndreyAkinshin AndreyAkinshin added this to the v0.12.1 milestone Mar 2, 2020
@AndreyAkinshin
Copy link
Member

@WojciechNagorski it seems that one of our integration tests (UserCanSpecifyCustomBuildConfiguration) is red after the merge, see https://ci.appveyor.com/project/dotnetfoundation/benchmarkdotnet/history
Could you please take a look?

@WojciechNagorski
Copy link
Contributor Author

Ok no problem. I will take a look tomorrow.

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.

3 participants