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.

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

Consider additional FxCop rules to enable #24055

Closed
javiercn opened this issue Jul 17, 2020 · 7 comments
Closed

Consider additional FxCop rules to enable #24055

javiercn opened this issue Jul 17, 2020 · 7 comments
Labels
affected-few This issue impacts only small number of customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-nice-to-have This label is used by an internal tool

Comments

@javiercn
Copy link
Member

This is a follow up issue from #9620 which added the infrastructure to enable some basic FxCop rules on the repository.

We want to come up with a list of issues so that we can discuss and decide which ones we want to enable and which ones we don't think add value or are not generally applicable enough to a repository this size.

The approach will be to propose a few rules at a time and introduce them based on general agreement so that we can progressively enable the rules we care about.

@javiercn javiercn added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jul 17, 2020
@Pilchie
Copy link
Member

Pilchie commented Jul 17, 2020

I believe @JamesNK proposed the globalization rules about using stringcomparers, Ordinal, etc as a good first step, and a case where we have already had some bugs to fix.

@JamesNK
Copy link
Member

JamesNK commented Jul 17, 2020

The other good validation is requiring ConfigureAwait(false) on awaits in projects that could run outside of ASP.NET Core. I don't know whether we have any of those in this repo.

(The .NET SignalR client doesn't count as it has its own strategy to avoid the need for ConfigureAwait(false))

@wtgodbe wtgodbe added affected-few This issue impacts only small number of customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-nice-to-have This label is used by an internal tool labels Nov 12, 2020 — with ASP.NET Core Issue Ranking
@mkArtakMSFT mkArtakMSFT added this to the 6.0-preview2 milestone Jan 26, 2021
@mkArtakMSFT mkArtakMSFT removed this from the 6.0-preview2 milestone Jan 28, 2021
@mkArtakMSFT
Copy link
Member

Putting this back to Next Sprint Planning in favor of #27631, as this will help community be successful with our repo quicker.

@mkArtakMSFT mkArtakMSFT added this to the 6.0-preview5 milestone Apr 20, 2021
@pranavkm
Copy link
Contributor

Quick note, we should be targeting NetAnalyzers that's part of the .NET SDK: https://docs.microsoft.com/en-us/visualstudio/code-quality/migrate-from-fxcop-analyzers-to-net-analyzers?view=vs-2019

@ghost ghost added the Working label May 19, 2021
pranavkm added a commit that referenced this issue May 26, 2021
pranavkm added a commit that referenced this issue May 26, 2021
pranavkm added a commit that referenced this issue May 26, 2021
* Enable CA1821, CA1825-CA1834

Contributes to #24055
pranavkm added a commit that referenced this issue Jan 27, 2022
pranavkm added a commit to pranavkm/aspnetcore that referenced this issue Feb 3, 2022
pranavkm added a commit that referenced this issue Feb 4, 2022
* Enable CA2007 for all ns2.0 projects

Contributes to #24055
pranavkm added a commit that referenced this issue Feb 5, 2022
* Refactor : Enable CA2249

1. Enables CA2249

Contributes to #24055
pranavkm added a commit to pranavkm/aspnetcore that referenced this issue Feb 24, 2022
pranavkm added a commit that referenced this issue Feb 25, 2022
Contributes to #24055
Contributes to #32087

* This updates most of the repo to use LoggerMessageAttributes. MvcCoreLoggerExtensions is the only standout but it is fairly involved since it mixes logging from several types, re-uses logger ids etc
pranavkm added a commit that referenced this issue Mar 2, 2022
* Enable IDE0060

Contributes to #24055
pranavkm added a commit to pranavkm/aspnetcore that referenced this issue Mar 19, 2022
@ghost
Copy link

ghost commented Nov 17, 2022

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@wtgodbe
Copy link
Member

wtgodbe commented Sep 11, 2024

Closing as I believe this is either no longer relevant, or has been superseded by 1ES infra

@wtgodbe wtgodbe closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-nice-to-have This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

7 participants
@pranavkm @JamesNK @Pilchie @javiercn @wtgodbe @mkArtakMSFT and others