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

Remove rule CA2109 #31659

Closed
wants to merge 4 commits into from
Closed

Remove rule CA2109 #31659

wants to merge 4 commits into from

Conversation

x789
Copy link
Contributor

@x789 x789 commented Oct 8, 2022

Removes documentation of rule CA2109.
Rule is removed by PR dotnet/roslyn-analyzers#6198.
Removal of the rule was discussed in dotnet/roslyn-analyzers#5974.

@x789 x789 requested review from gewarren and a team as code owners October 8, 2022 08:11
@ghost ghost added the community-contribution Indicates PR is created by someone from the .NET community. label Oct 8, 2022
@x789 x789 changed the title Remove CA2109 (roslyn-analyzers #5974) Remove rule CA2109 (roslyn-analyzers #5974) Oct 8, 2022
@x789 x789 changed the title Remove rule CA2109 (roslyn-analyzers #5974) Remove rule CA2109 Oct 8, 2022
@gewarren gewarren added the 🚧 Hold for related PR Indicates a PR can only be merged when other related PRs are merged (see comments for links) label Oct 10, 2022
@gewarren
Copy link
Contributor

We'll at least need to add a redirection to https://github.com/dotnet/docs/blob/main/.openpublishing.redirection.fundamentals.json. But I'm wondering if we should also add a new page/section for "deleted" or "obsolete" rules, just so some reference remains. Thoughts?

@Youssef1313
Copy link
Member

When CA1801 got deprecated, a note was just added. See #21882

Regardless of the decision, we should be consistent.

@gewarren
Copy link
Contributor

When CA1801 got deprecated, a note was just added. See #21882

Regardless of the decision, we should be consistent.

Yes, I like that better. Leave the article in place but add a prominent banner at the top like #21882.

@x789
Copy link
Contributor Author

x789 commented Oct 11, 2022

I like the idea that information about the rule is not deleted completely.
From a developer's point of view, the reason why a rule was removed is definitely helpful.

I will adjust the PR so that the information for CA2109 is not removed, but a banner is added (as suggested by @Youssef1313).

Due to my current workload, this may take some days.

@x789
Copy link
Contributor Author

x789 commented Oct 16, 2022

I started over with a new branch to get a clean commit history. See PR #31797.

@x789 x789 closed this Oct 16, 2022
@x789 x789 deleted the remove-ca2109 branch October 16, 2022 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gewarren gewarren Awaiting requested review from gewarren gewarren is a code owner

Assignees
No one assigned
Labels
community-contribution Indicates PR is created by someone from the .NET community. 🚧 Hold for related PR Indicates a PR can only be merged when other related PRs are merged (see comments for links)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@x789 @gewarren @Youssef1313