Repository navigation
Enable CA1822 (Mark members as static) analyzer - #66333
Conversation
|
Tagging subscribers to this area: @dotnet/area-meta Issue Detailsnull
|
There was a problem hiding this comment.
@krwq, is any of this tracing stuff ever used? Wondering if we can just delete it all instead.
dcf6f93 to
2c165a0
Compare
| } | ||
|
|
||
| private void OnIndexedHeaderNamePostBase(int index) | ||
| private static void OnIndexedHeaderNamePostBase(int index) |
There was a problem hiding this comment.
These won't be static once they're actually implemented.
There was a problem hiding this comment.
I think that's fine, one you implemented them you remove static
| @@ -10,9 +10,9 @@ internal sealed class DefaultSettingsSection // ConfigurationSection | |||
|
|
|||
| internal static DefaultSettingsSection GetSection() => s_section; | |||
|
|
|||
| public string DistributedTransactionManagerName { get; set; } = ConfigurationStrings.DefaultDistributedTransactionManagerName; | |||
| public static string DistributedTransactionManagerName { get; set; } = ConfigurationStrings.DefaultDistributedTransactionManagerName; | |||
There was a problem hiding this comment.
@roji should weigh in on whether any of these changes are going to conflict with his work.
| } | ||
|
|
||
| private void OnIndexedHeaderNamePostBase(int index) | ||
| private static void OnIndexedHeaderNamePostBase(int index) |
There was a problem hiding this comment.
I believe this file is mirrored with aspnetcore. It'd be good to validate whether it's going to break anything when that happens.
There was a problem hiding this comment.
I believe this should be fine as it's private and aspnetcore already had them static (they have CS1822 enabled) before sync with runtime removed it.
stephentoub
left a comment
There was a problem hiding this comment.
Other than the remaining comments, LGTM. (I don't plan to re-review the 642 files again ;-)
No description provided.