[Build Server] Automated Style Checking and Feedback

[robertkety]

I'd like to determine a means to have the build server scan incoming file changes against a style policy. Furthermore, I'd like the policy to distinguish between style requirements for our repository and style guidance we'd prefer contributors to follow. The distinction between the two types of policies determines if the pull request is rejected automatically or accepted with warnings. A pull request with no style violations or warnings (and a valid build) would be considered a pass and simply await review, authorization, and merge. The weighted style policy would allow us to control critical style behavior (e.g., class order of fields, properties, methods, indexers, etc.) while not pestering contributors with nice-to-have style behavior (e.g., white space after semi-colon).
The determination of what qualifies as style guidance versus style requirement may happen at a later time, but a good resource might be found by examining the capabilities of 'Format Document' in Visual Studio 2015 (Edit->Advanced->Format Document OR Ctrl-K, Ctrl-D).

[persn]

DotNetAnalyzers/StyleCopAnalyzers has weighted differences if I understand your criteria of weighted differences correctly.

Just like ReSharper does we can easily set certain rules to "Info" so that the IDE will suggest changes, but not enforce them with warnings or errors.

[persn]

We can also set rules to pass warnings and errors, and coupled with "Pass warnings as errors" as in #124 we get CI coverage for style checking.

[persn]

I'm experimenting with DotNetAnalyzers/StyleCopAnalyzers in a local branch, I may have stuff to show off tonight or tomorrow.

[persn]

#159 demonstrates that we now receive feedback on style checks with CI builds. Is this satisfactory for the scope of this issue?

[robertkety]

Can we do a test that demonstrates the warning condition (where the pr is not rejected, but contains style changes we'd prefer contributions not have)?

[persn]

We absolutely cannot, how would that even work? The reason the current test works is because we're utilizing the "warnings as errors" attributes in our C# projects. If the StyleCop rule is set to be a warning, it will result in a failed error.

[persn]

It doesn't matter, I'm been trying it out now, and I've decided the "warning-as-errors" in combination with StyleCop is just way too annoying to be considered coder friendly so I need to remove it.

[persn]

@robertkety Do we really want the use case you described? Automated feedback on warnings, but not reject the Pull Request? I just don't understand what the purpose is, I can't think of a realistic situation where a warning will show on GitHub, and I won't ask the contributor to fix it. Every warning not fixed will carry over to the next PR, bothering other contributors who didn't even introduce the warning in the first place.

In my mind the perfect scenario of what we want is: When the contributor is coding on their computer they will get Error, Warning and Info messages like usual, nothing special about it, this will give the contributor feedback in a non-hostile way and let them code without being hindered by automatic StyleCop blocks and whatever else. However on the build server any config should fail if any Errors or Warnings are found, I repeat that any warnings not fixed will carry over and annoy others.

I'm not sure exactly how to achieve this, but I think it should be possible somehow to toggle Werror(Warning on Error) so that it is only active when on a build server, it's worth looking into. EDIT: The easiest way to achieve this is to toggle Werror for Release builds only I suppose.