Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delete IDE dispose analyzers #50303

Merged
merged 3 commits into from
Jan 7, 2021
Merged

Delete IDE dispose analyzers #50303

merged 3 commits into from
Jan 7, 2021

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Jan 7, 2021

  • These rules have been disabled by default for more than a year now
  • These rules were ported from CA rules to IDE rules to enable shipping them in the box. This is no longer required as all CA and IDE rules ship with the .NET SDK now.

Closes #38984
Closes #38944
Closes #38204
Closes #36507
Closes #38900

@mavasani
Delete IDE dispose analyzers
da95368 
- These rules have been disabled by default for more than a year now
- These rules were ported from CA rules to IDE rules to enable shipping them in the box. This is no longer required as all CA and IDE rules ship with the .NET SDK now.

Closes dotnet#38984
Closes dotnet#38944
Closes dotnet#38204
Closes dotnet#36507
Closes dotnet#38900
@mavasani mavasani added this to the 16.9 milestone Jan 7, 2021
@mavasani mavasani marked this pull request as ready for review January 7, 2021 14:13
@mavasani mavasani requested a review from a team as a code owner January 7, 2021 14:13
@Youssef1313
Copy link
Member

From #38900 (comment):

📝 Note that IDE0069 and other diagnostics are not intended to directly duplicate prior CA* warnings. The new rules are reviewed for ongoing correctness against current practices, and may be adjusted in various ways.

This seem to conflict with the reasoning of removal:

These rules were ported from CA rules to IDE rules to enable shipping them in the box. This is no longer required as all CA and IDE rules ship with the .NET SDK now.

cc: @sharwell

@mavasani
Copy link
Contributor Author

mavasani commented Jan 7, 2021 •
edited
Loading

@Youssef1313 The IDE dispose rules were mostly a port from CA rules. It may have had some fine tuning done with time, but they are almost the same. These are code quality rules, and do not belong to the CodeStyle/Unnecessary code rules present in Roslyn repo. If at all we find cases that need improvement, they should be done to the CA rules.

genlu
genlu approved these changes Jan 7, 2021
View reviewed changes
Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

:shipit:

@mavasani mavasani merged commit eeba499 into dotnet:master Jan 7, 2021
@mavasani mavasani deleted the DeleteDisposeAnalyzers branch January 7, 2021 19:04
@ghost ghost modified the milestones: 16.9, Next Jan 7, 2021
@Cosifne Cosifne modified the milestones: Next, 16.9 Jan 27, 2021
@JoeRobich JoeRobich modified the milestones: 16.9, 16.9.P4 Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Youssef1313 Youssef1313 Youssef1313 left review comments

@jmarolf jmarolf jmarolf approved these changes

@genlu genlu genlu approved these changes

Assignees
No one assigned
Labels
Area-IDE
Projects
None yet
Milestone
16.9.P4
6 participants
@mavasani @Youssef1313 @genlu @jmarolf @JoeRobich @Cosifne