-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Code style rules cleanup #19801
Code style rules cleanup #19801
Conversation
Removed some obsolete entries now that there is a new build directory.
It currently has two functions, both of which are covered by our .editorconfig file.
910d64e
to
70aacdc
Compare
The initial goal of making ReSharper respect our non-standard rule of omitting explicit access modifiers is looking like a failure, but the cleanup is still worth and we now get CI enforcement of that rule, so overall I rate this a win. P.S.: The misleading warning text for explicit access modifiers still applies, but that's the best we're going to get until the .NET people fix it. |
- The DEFAULT_PRIVATE_MODIFIER behaviour is now handled by the .editorconfig file via `dotnet_style_require_accessibility_modifiers = omit_if_default:warning`. Also added `dotnet_diagnostic.IDE0040.severity = warning` there to raise compile-time errors in the CI. - The field naming conventions seem to already be covered by (some) analyzer rules (checked in both VS and VSCode) - IDE1006/SA1306 and SA1307.
70aacdc
to
992282a
Compare
Fixed the new comments in |
Between
.editorconfig
OpenRA.ruleset
OpenRA.sln.DotSettings
stylecop.json
I felt our code style rules configuration is all over the place. So I took a peek inside them and it turns out two of them have been made redundant over the years.
stylecop.json
only contains generic file formatting rules that are already(/now) covered by.editorconfig
OpenRA.sln.DotSettings
contains C# field naming rules that are already covered by code analyzers and a rule about default access modifiers, that we can cover with an.editorconfig
settingAlso removed some obsolete entries from
.gitignore
.(I am going to try and sneak this onto the release milestone. If it proves problematic somehow we can remove it.)