Skip to content

Enable CA1012#52362

Merged
sharwell merged 3 commits intodotnet:mainfrom
Youssef1313:patch-4
Apr 6, 2021
Merged

Enable CA1012#52362
sharwell merged 3 commits intodotnet:mainfrom
Youssef1313:patch-4

Conversation

@Youssef1313
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 commented Apr 2, 2021

Related to #52359

@ghost ghost added the Area-Infrastructure label Apr 2, 2021
@Youssef1313 Youssef1313 marked this pull request as ready for review April 2, 2021 18:39
@Youssef1313 Youssef1313 closed this Apr 2, 2021
@Youssef1313 Youssef1313 reopened this Apr 2, 2021
@Youssef1313 Youssef1313 force-pushed the patch-4 branch 2 times, most recently from 91114ea to a0d33dc Compare April 3, 2021 10:11
@Youssef1313 Youssef1313 requested a review from a team as a code owner April 3, 2021 10:11
@Youssef1313 Youssef1313 marked this pull request as draft April 4, 2021 15:56
@Youssef1313 Youssef1313 marked this pull request as ready for review April 4, 2021 20:48
@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. and removed Area-Infrastructure labels Apr 5, 2021
Comment thread .editorconfig Outdated
Comment on lines +263 to +264
# CA1012: Abstract types should not have constructors
dotnet_diagnostic.CA1012.severity = warning
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.

❔ Can we move this to one of the .globalconfig files? The parsing performance of those files is better than .editorconfig, and they are more consistently applied to the code base.

Copy link
Copy Markdown
Contributor

@mavasani mavasani Apr 5, 2021

Choose a reason for hiding this comment

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

If we feel parsing performance is an issue, can we file a compiler bug for it to improve editorconfig performance? Also we should consider moving everything over from editorconfig to globalconfig instead of just one off changes.

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.

If we feel parsing performance is an issue, can we file a compiler bug for it to improve editorconfig performance?

The .editorconfig parsing performance is very good, but by definition has to be applied per-file. .globalconfig doesn't need to consider paths. The latter is also preferable because duplication definitions trigger a warning rather than silently ignoring one of them.

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.

@sharwell Can this be tracked separately for everything in editorconfig as suggested by @mavasani?

Comment thread .editorconfig Outdated
Co-authored-by: Manish Vasani <mavasani@microsoft.com>
@sharwell sharwell merged commit 4e4936a into dotnet:main Apr 6, 2021
@ghost ghost added this to the Next milestone Apr 6, 2021
@Youssef1313 Youssef1313 deleted the patch-4 branch April 6, 2021 06:15
@dibarbet dibarbet modified the milestones: Next, 16.10.P3 Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants