Skip to content

Add support for .editorconfig files to MSBuildWorkspace#35848

Merged
jasonmalinowski merged 2 commits intodotnet:masterfrom
jasonmalinowski:add-editorconfig-support-to-msbuildworkspace
Jun 5, 2019
Merged

Add support for .editorconfig files to MSBuildWorkspace#35848
jasonmalinowski merged 2 commits intodotnet:masterfrom
jasonmalinowski:add-editorconfig-support-to-msbuildworkspace

Conversation

@jasonmalinowski
Copy link
Copy Markdown
Member

Just threading through files discovered to the workspace; no interesting changes here.

Just threading through files discovered to the workspace; no interesting
changes here.
@jasonmalinowski jasonmalinowski requested a review from a team as a code owner May 21, 2019 18:25
{
var files = GetSimpleCSharpSolutionFiles()
.WithFile(@"CSharpProject\CSharpProject.csproj", Resources.ProjectFiles.CSharp.WithDiscoverEditorConfigFiles)
.ReplaceFileElement(@"CSharpProject\CSharpProject.csproj", "DiscoverEditorConfigFiles", "false")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasonmalinowski Are we planning to provide an API on ProjectInfo to query the value of DiscoverEditorConfigFiles for a given project? For example, I think I need to guard the code action being added in #35691 to only show up if DiscoverEditorConfigFiles = true.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't planning on it -- the flag is more a feature flag than anything else and we only expect people where it's a problem to turn it off once we roll the feature out.

Why would that code action only work if discovery is on? If discovery is off either (a) you have no .editorconfig files, or (b) you have discovery off but manually specified some in your project. In the (b) case the code fix should still work, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would that code action only work if discovery is on?

If the code action adds an entry dotnet_diagnostic.CAxxxx.severity = severity, will it be respected? I think for the code style based severity configuration, it will still be respected in live analysis.
Regardless, I presume I need to test this scenario more thoroughly to ensure sanity.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be respected if [a] there are analyzer config documents in the project (even if they weren't discovered nicely) and [b] you have the feature flag that @JoeRobich is updating.

Our CI machines don't have a new enough MSBuild to support this; adding
it is tracked by dotnet/core-eng#6424 which
is currently underway but there's no reason to hold up the larger
work on the test.
@jasonmalinowski jasonmalinowski merged commit f0be63d into dotnet:master Jun 5, 2019
@jasonmalinowski jasonmalinowski deleted the add-editorconfig-support-to-msbuildworkspace branch June 5, 2019 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants