Convert .ruleset to .globalconfig #49181
Conversation
sharwell
force-pushed
the
globalconfig
branch
from
fb29d12 to
0ea2492
Compare
eng/targets/Imports.targets
Outdated
| <!-- Always include Common.globalconfig --> | ||
| <EditorConfigFiles Include="$(RepositoryEngineeringDir)config\rulesets\Common.globalconfig" /> | ||
| <!-- Include Shipping.globalconfig for shipping projects --> | ||
| <EditorConfigFiles Condition="'$(IsShipping)' == 'true'" Include="$(RepositoryEngineeringDir)config\rulesets\Shipping.globalconfig" /> |
There was a problem hiding this comment.
Tagging @chsienki - I believe including these as GlobalAnalyzerConfigFiles should work and make it more clear that these are global analyzer configs, not editorconfigs.
There was a problem hiding this comment.
There are two problems with this approach:
-
This repository currently uses a compiler build that does not support this property
-
The conversion of
GlobalAnalyzerConfigFilestoEditorConfigFilesis conditioned on$(DiscoverGlobalAnalyzerConfigFiles):We would need to remove this condition (and wait for it to apply to the SDK used for this repository) before
GlobalAnalyzerConfigFilesbecomes a viable alternative toEditorConfigFiles.
There was a problem hiding this comment.
Confirmed the property GlobalAnalyzerConfigFiles works as expected: ClassLibrary58.zip
There was a problem hiding this comment.
Confirmed the property GlobalAnalyzerConfigFiles works as expected
It might work in newer SDKs, but it doesn't work in the version of the SDK and/or compiler currently specified for Roslyn command line builds.
It also doesn't address the fact that setting unrelated property DiscoverGlobalAnalyzerConfigFiles currently disables the manual inclusion of global analyzer config files.
There was a problem hiding this comment.
DiscoverGlobalAnalyzerConfigFiles is not defaulted anywhere as per my knowledge, so the condition should be true by default and current logic should work.
There was a problem hiding this comment.
It also doesn't address the fact that setting unrelated property DiscoverGlobalAnalyzerConfigFiles currently disables the manual inclusion of global analyzer config files.
Yes, that seems reasonable - @chsienki I can file a tracking bug for you if you agree.
| @@ -0,0 +1,99 @@ | |||
| is_global = true | |||
|
|
|||
| dotnet_diagnostic.CA1067.severity = warning | |||
There was a problem hiding this comment.
Note: I did not validate that each of the entries in the global configs map to the entries in current rulesets.
| @@ -1,95 +0,0 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <RuleSet Name="Common diagnostic rules to run during build for all shipping Roslyn projects" Description="This file contains diagnostic settings used by all Roslyn projects. Projects that need specific settings should have their own rule set files that Include this one, and then make the necessary adjustments." ToolsVersion="14.0"> | |||
ghost
added this to the 
No description provided.