microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.06k stars 115 forks source link

Adds an .editorconfig file, formats, and resolves analyzer issues #436

Closed rossgrambo closed 4 months ago

rossgrambo commented 7 months ago

Why this PR?

What was done?

  1. Added a .editorconfig file to the solution a. This is modified from the default .editorconfig generated by Visual Studio.
  2. Ran dotnet format
  3. Ran Visual Studio .NET code analyzer & manually resolved issues
  4. Updated FeatureManagement and FeatureManagement.AspNetCore with EnforceCodeStyleInBuild: true. This means warnings will show in a build when the code style is not being followed.

Adding an .editorconfig enables code formatting and code analyzing tools to work. Simply by adding this file, Visual Studio and other IDEs will now respect the style choices for this project when making changes within it. Additionally, this unlocks GitHub actions that can evaluate code style and formatting and catch small mistakes for us.

Visible Changes

No visible changes

Philosophy

Code styles should never feel like a hurdle or a headache that we need to adhere to- so at no point should these changes block or slow development. However I believe adding this file allows for a clear definition of rules, a clear place to change style choices as we see fit, and enables IDEs & tooling to quickly follow suit.

This also means all other PRs will be cleaner- as any code style issues will already be caught and handled by tooling.

zhiyuanliang-ms commented 7 months ago

This PR is great! This repo has been missing lint checks for a long time. The format check should be added to the azure pipeline.

jimmyca15 commented 7 months ago

This is the type of change that is cross-repo. If we were to adopt this, all repos in our scope should adopt it.

zhiyuanliang-ms commented 6 months ago

Adjusted the AnalysisLevel in csproj file to suppress CA1848 and CA2254 warnings: image

Now, the AnalysisLevel for Microsoft.FeatureManagement is 5.0 because Microsoft.FeatureManagement is targeted on netstandard and 5.0 is the lowest AnalysisLevel which was available for the .NET 5 release is used.

the AnalysisLevel for Microsoft.FeatureManagement.AspNetCore is 6.0 because Microsoft.FeatureManagement.AspNetCore is targeted on .NET 6.0.

zhiyuanliang-ms commented 6 months ago

A reference for all the format rules: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/csharp-formatting-options

zhiyuanliang-ms commented 4 months ago

Move to #472