Enforce additional style rules. #20726
Conversation
RoosterDragon
force-pushed
the
style-easy
branch
from
f40bba6
to
52b9dd4
Compare
|
Apart from very inconsistent punctuation LGTM |
.editorconfig
Outdated
| dotnet_naming_symbols.const_fields.applicable_accessibilities = * | ||
| dotnet_naming_symbols.const_fields.required_modifiers = const | ||
| # IDE0040 Add accessibility modifiers | ||
| dotnet_style_require_accessibility_modifiers = omit_if_default #for_non_interface_members |
There was a problem hiding this comment.
#for_non_interface_members is that to be enabled in the future?
There was a problem hiding this comment.
No, it's a comment to indicate the default value of the setting. Since OpenRA prefers to have implicit accessibility eveyrwhere, we change to omit_if_default instead.
There was a problem hiding this comment.
Is there really a reason to list the default value? I don't see the benefit of that and it is just confusing because you don't know if that is a future TODO.
There was a problem hiding this comment.
If something is a TODO then it will/should say "TODO: ...".
But I do agree specifying the default values that we override is redundant.
There was a problem hiding this comment.
Removed the comments when defaults overridden.
There was a problem hiding this comment.
if we aren't worried that this will become a maintenance overhead, I'm all for displaying default values. It saves a bunch of hassle when working with rules
There was a problem hiding this comment.
What is your concrete usecase that would be a hassle? If we don't use the default then there is a reason for it. And if not this should be a TODO.
There was a problem hiding this comment.
it's great for when we want to change rules again, whenever that will be
RoosterDragon
force-pushed
the
style-easy
branch
from
5b904e8
to
feff88a
Compare
.editorconfig
Outdated
| dotnet_naming_symbols.const_fields.applicable_accessibilities = * | ||
| dotnet_naming_symbols.const_fields.required_modifiers = const | ||
| # IDE0040 Add accessibility modifiers | ||
| dotnet_style_require_accessibility_modifiers = omit_if_default #for_non_interface_members |
There was a problem hiding this comment.
If something is a TODO then it will/should say "TODO: ...".
But I do agree specifying the default values that we override is redundant.
|
Oooh, also has an actual conflict in |
RoosterDragon
force-pushed
the
style-easy
branch
from
feff88a
to
032011f
Compare
RoosterDragon
force-pushed
the
style-easy
branch
2 times, most recently
from
0fad083
to
6f62f20
Compare
- Group style rules with their associated options in a way that matches the documentation. This makes it easier to pair rules and their options. - Remove OpenRA.ruleset and move all rules into .editorconfig file. - Centralise IDE0005 workaround in Directory.Build.props file.
RoosterDragon
force-pushed
the
style-easy
branch
from
6f62f20
to
794c0e3
Compare
Enforce another selection of style rules. These have no existing violations so no code changes needed - just need to decide if we're happen to apply these or not. See docs for details of each rule: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/
As requested, also reformats the editorconfig file to make it friendlier to edit: