reactiveui / rfcs

RFCs for changes to ReactiveUI
https://reactiveui.net/rfcs
5 stars 5 forks source link

RFC: Turn on Analyzers #18

Closed glennawatson closed 5 years ago

glennawatson commented 5 years ago

Now that the main ReactiveUI repository is down to 0 pull requests I want to take the opportunity to mostly do a non-public changing refactor to turn on three different analyzers

The stylecop ones enforce Documentation for all our public facing code (an issue for our website) and will keep code consistent. The stylecop analyzer can enforce two different class variables naming scheme, that is where we have no prefix on the variable name, and use "this.myClassVariable = 20;" or use a _ prefix, eg "_myClassVariable = 20;"

The Microsoft FX Cop analyzers have a bunch of analyzers that used to live in the static analyzers, they tend to pick up things like not disposing objects correctly, Very common code mistakes. Some localization stuff as well.

The Roslynator Analyzers pick up very common coding practices, like use Method Groups instead of Lambda for example. These emulate a lot of the analysis that Resharper does.

The normal main point with these types of suggestions is they make PRs harder so something we can only do while our PR number is 0.

glennawatson commented 5 years ago

Also I guess if anyone has any feedback about coding styles, we seem to more adopt the _variable style in our code base currently so may be less pressure.

This is consistent with other microsoft code bases for example https://github.com/aspnet/Home/wiki/Engineering-guidelines#coding-guidelines

GitHub
aspnet/Home
The Home repository is the starting point for people to learn about ASP.NET Core.
glennawatson commented 5 years ago

This one would from a external users point of view main advantage will be better API documentation on the website. The internal is more consistent code, and trivial to detect errors picked up.

glennawatson commented 5 years ago

Also I hope not to turn this into a coding holy war, so the stylecop rules are pretty flexible based on our the community feels about it etc.

glennawatson commented 5 years ago

Fixed by https://github.com/reactiveui/ReactiveUI/pull/1749