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

Relate coding conventions to diagnostics and editorconfig settings #33341

Closed
BillWagner opened this issue Jan 5, 2023 · 2 comments
Closed

Relate coding conventions to diagnostics and editorconfig settings #33341

BillWagner opened this issue Jan 5, 2023 · 2 comments
Assignees
Labels
dotnet-csharp/svc fundamentals/subsvc okr-freshness OKR: Freshness of content 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@BillWagner
Copy link
Member

BillWagner commented Jan 5, 2023

This article contains a set of guidelines that have different levels of adoption across the C# community. Some are almost universal (interfaces start with a capital I). Some are more common, but not universal (starting fields with _). Some have fallen out of favor in recent times.

This page needs to be updated to reflect current styles.

Furthermore, we should categorize coding conventions to the kinds of diagnostics available:

  • Compiler warnings can help developers remove potential coding mistakes. For example CS0642: Possible mistaken empty statement.
  • Analyzers find other potential mistakes, including performance or security risks.
  • Style can be enforced using editorconfig settings.

In addition, we should reference the preferred style used by the runtime. That's a common style seen by .NET developers browsing our source, and it's generally consistent with what we use in docs.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.


Associated WorkItem - 58354

@BillWagner
Copy link
Member Author

While doing this work, note that this article has a recommendation that local functions should be PascalCased. That's the current recommendation, because they are methods. However, the justification is that they are "public", like classes, interfaces and structs. That justification is incorrect.

We've generally preferred PascalCasing for methods because we chose PascalCase for all methods, regardless of access. We think of local functions as functions first. The fact they their scope is local is less important for the name.

@BillWagner BillWagner self-assigned this Jan 6, 2023
@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label Jan 6, 2023
@BillWagner BillWagner added ⌚ Not Triaged Not triaged 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Jan 6, 2023
@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label Jan 6, 2023
@github-actions github-actions bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Jan 6, 2023
@BillWagner BillWagner moved this from 🔖 Ready to Slipped in dotnet/docs June 2023 sprint Jun 30, 2023
@BillWagner BillWagner added 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. okr-freshness OKR: Freshness of content dotnet-csharp/svc and removed doc-enhancement dotnet-csharp/svc fundamentals/subsvc labels Dec 4, 2024
@BillWagner BillWagner removed the 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. label Jan 6, 2025
@BillWagner BillWagner moved this from 🔖 Ready to 🏗 In progress in dotnet/docs January 2025 sprint project Jan 14, 2025
@dotnetrepoman dotnetrepoman bot added the 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. label Jan 14, 2025
@dotnet-policy-service dotnet-policy-service bot removed the 🗺️ mapQUEST Only used as a way to mark an issue as updated for quest. RepoMan should instantly remove it. label Jan 14, 2025
@BillWagner
Copy link
Member Author

This has already been addressed in #33341

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in dotnet/docs January 2025 sprint project Jan 14, 2025
BillWagner added a commit to BillWagner/docs that referenced this issue Jan 14, 2025
- dotnet#33341 already addressed in dotnet#36428.
- Fixes dotnet#37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes dotnet#37295: Don't use `ID` in the sample.
- Fixes dotnet#37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes dotnet#41748: Change the sample so the constructor is relevant.
BillWagner added a commit to BillWagner/docs that referenced this issue Jan 14, 2025
- dotnet#33341 already addressed in dotnet#36428.
- Fixes dotnet#37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes dotnet#37295: Don't use `ID` in the sample.
- Fixes dotnet#37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes dotnet#41748: Change the sample so the constructor is relevant.
BillWagner added a commit to BillWagner/docs that referenced this issue Jan 14, 2025
- dotnet#33341 already addressed in dotnet#36428.
- Fixes dotnet#37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes dotnet#37295: Don't use `ID` in the sample.
- Fixes dotnet#37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes dotnet#41748: Change the sample so the constructor is relevant.
BillWagner added a commit to BillWagner/docs that referenced this issue Jan 14, 2025
- dotnet#33341 already addressed in dotnet#36428.
- Fixes dotnet#37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes dotnet#37295: Don't use `ID` in the sample.
- Fixes dotnet#37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes dotnet#41748: Change the sample so the constructor is relevant.
- Fixes dotnet#42858: Add an explanation on declaring variables in top level statements.
BillWagner added a commit to BillWagner/docs that referenced this issue Jan 15, 2025
- dotnet#33341 already addressed in dotnet#36428.
- Fixes dotnet#37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes dotnet#37295: Don't use `ID` in the sample.
- Fixes dotnet#37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes dotnet#41748: Change the sample so the constructor is relevant.
- Fixes dotnet#42858: Add an explanation on declaring variables in top level statements.
BillWagner added a commit that referenced this issue Jan 17, 2025
* Fix small open issues.

- #33341 already addressed in #36428.
- Fixes #37294: Add text that the shortened format is valid only when the runtime type matches the variable type.
- Fixes #37295: Don't use `ID` in the sample.
- Fixes #37296: Fix nullable warnings. Other issue comments are incorrect.
- Fixes #41748: Change the sample so the constructor is relevant.
- Fixes #42858: Add an explanation on declaring variables in top level statements.

* Remaining open issues

- Fixes #43838: Add notes for collection expressions and using primary constructors. Include naming recommendations.
- Fixes #43839  Update the overview of constructors to include information on primary constructors.

* Code review for style

Fix any style issues with the current code.

* Final edit pass

I also caught a couple sample bits that I'd neglected in the previous commit.

* Apply suggestions from code review

Co-authored-by: Genevieve Warren <[email protected]>

---------

Co-authored-by: Genevieve Warren <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet-csharp/svc fundamentals/subsvc okr-freshness OKR: Freshness of content 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
Status: Slipped
Development

No branches or pull requests

2 participants