Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add global AnalyzerConfig with default configuration#13835

Merged
iSazonov merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:add-analyzers-props2xtqqczze/PowerShell-PowerShell:add-analyzers-props2Copy head branch name to clipboard
Oct 26, 2020
Merged

Add global AnalyzerConfig with default configuration#13835
iSazonov merged 3 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:add-analyzers-props2xtqqczze/PowerShell-PowerShell:add-analyzers-props2Copy head branch name to clipboard

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Oct 22, 2020

Copy link
Copy Markdown
Contributor

PR Summary

  • Copy microsoft.codeanalysis.netanalyzers/5.0.0-rtm.20502.2/editorconfig/AllRulesDefault/.editorconfig to .globalconfig
  • Add Add IDE diagnostics to .globalconfig with most rules hidden
    • copied from dotnet/runtime and converted using RulesetToEditorconfigConverter.exe
  • Disable diagnostics for Microsoft.Management.UI.Internal

PR Context

Alternative to #12892

If Visual Studio 2019 is installed, it should be updated to 16.8 Preview5+ because OmniSharp appears to prefer the Visual Studio MSBuild instance to the StandAlone instance. Otherwise, the global AnalyzerConfig may be ignored.

PR Checklist

@iSazonov

iSazonov commented Oct 22, 2020

Copy link
Copy Markdown
Collaborator

Why not follow https://github.com/dotnet/runtime/blob/master/eng/Analyzers.props?
(Also I'd think about xUnit tests folder too.)

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

@xtqqczze

xtqqczze commented Oct 22, 2020

Copy link
Copy Markdown
Contributor Author

Why not follow https://github.com/dotnet/runtime/blob/master/eng/Analyzers.props?
(Also I'd think about xUnit tests folder too.)
Previously, you had wanted to use EditorConfig instead of ruleset configuration.

GitHub
.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps. - dotnet/runtime

@iSazonov

Copy link
Copy Markdown
Collaborator

Previously, you had wanted to use EditorConfig instead of ruleset configuration.

My thought was and is that we shouldn't have two configs - in EditorConfig and in ruleset. If VS Code supports(?) ruleset it is better to use ruleset and follow .Net team's pattern.

@xtqqczze

This comment has been minimized.

@xtqqczze xtqqczze closed this Oct 22, 2020
@xtqqczze xtqqczze reopened this Oct 23, 2020
@xtqqczze xtqqczze force-pushed the add-analyzers-props2 branch from 2a3256e to d90a6f9 Compare October 23, 2020 16:35
Add `microsoft.codeanalysis.netanalyzers\5.0.0-rtm.20502.2\editorconfig\AllRulesDefault\.editorconfig`
@xtqqczze xtqqczze force-pushed the add-analyzers-props2 branch from d90a6f9 to 13d0258 Compare October 23, 2020 17:03
@xtqqczze xtqqczze changed the title Add default global AnalyzerConfig Add global AnalyzerConfig with default configuration Oct 23, 2020
@xtqqczze xtqqczze marked this pull request as ready for review October 23, 2020 17:05
@xtqqczze

Copy link
Copy Markdown
Contributor Author

@iSazonov This PR uses .globalconfig file. It is the easiest solution and it just works, including TreatWarningsAsErrors.

@iSazonov iSazonov added the CL-Tools Indicates that a PR should be marked as a tools change in the Change Log label Oct 26, 2020
@iSazonov

Copy link
Copy Markdown
Collaborator

I will merge because:

  • we should benefit from Roslyn analyzers that is best practice.
  • it is more convenient than alternative analyzers.
  • we will be able to force rules in PRs without requests from maintainers.
  • After we moved to .Net 5.0 RC1 Roslyn analyzers are already enabled by default.
  • VS Code OmniSharp works with EditorConfig and GlobalConfig (need latest VS 2019 16.8 Preview5+).

As a side notice, we could disable Analyzers in all CIs to speed up compiling and separate check (compile only) with enabled analyzers.

@iSazonov iSazonov merged commit ab378f6 into PowerShell:master Oct 26, 2020
@iSazonov iSazonov added this to the 7.2.0-preview.1 milestone Oct 26, 2020
@xtqqczze xtqqczze deleted the add-analyzers-props2 branch October 26, 2020 16:22
@ghost

ghost commented Nov 17, 2020

Copy link
Copy Markdown

🎉v7.2.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Tools Indicates that a PR should be marked as a tools change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.