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

Fix naming violations in sample code (_camelCase with underscore for private fields) #30533

Closed
hakenr opened this issue Oct 1, 2023 · 6 comments
Assignees
Labels
Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@hakenr
Copy link
Member

hakenr commented Oct 1, 2023

The "new" official naming rules and conventions for C# identifiers require using _camelCase for private instance fields.

Blazor section of the documentation (especially in @code sections) does not comply with this standard.
E.g.

<PageTitle>Counter</PageTitle>

<h1>Counter</h1>

<p role="status">Current count: @currentCount</p>

<button @ref="myButton" class="btn btn-primary" @onclick="IncrementCount">Click me</button>

@code {
    private int currentCount = 0;
    private ElementReference myButton;

    private void IncrementCount()
    {
        currentCount++;
    }
}

Should be

<PageTitle>Counter</PageTitle>

<h1>Counter</h1>

<p role="status">Current count: @_currentCount</p>

<button @ref="_myButton" class="btn btn-primary" @onclick="IncrementCount">Click me</button>

@code {
    private int _currentCount = 0;
    private ElementReference _myButton;

    private void IncrementCount()
    {
        _currentCount++;
    }
}

Please confirm the "new" convention is expected to be used in Blazor or create a Blazor-specific sub-convention.

If you confirm the convention for Blazor, I can create a PR fixing all the occurences in the doc.


Document Details

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

  • ID: 5bc28995-6784-a7d2-5c0b-9811a8d19c1a
  • Version Independent ID: 1668c8fb-024c-c86d-d4d5-e22e3355c04b
  • Content: ASP.NET Core Blazor
  • Content Source: aspnetcore/blazor/index.md
  • Product: aspnet-core
  • Technology: aspnetcore-blazor
  • GitHub Login: @guardrex
  • Microsoft Alias: riande
@guardrex
Copy link
Collaborator

guardrex commented Oct 1, 2023

Hello @hakenr ... We go by the ASP.NET Core engineering guidelines without underscores, except that we use the this keyword in docs. They don't use this because they have a framework requirement to avoid it. They told me that we'll use this without underscores for all Blazor example code. I'll discuss this further with management. My impression from early discussions on this subject with the product unit is that the C# Guide specifications are more like non-binding guidelines and conventions, but senior management may have changed their minds about that over the years. The decision was made about six years ago ... and that's just about forever in framework time 👴😄. We might change over. I'll get back to you on this soon .............

@guardrex guardrex self-assigned this Oct 1, 2023
@guardrex guardrex added Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue labels Oct 1, 2023
@hakenr
Copy link
Member Author

hakenr commented Oct 1, 2023

Conventions are generally non-binding guidelines intended to enhance readability and ensure a consistent experience. Previously, there was no explicit naming convention for _privateField or privateField documented in Microsoft's official guidelines. However, this has now been formalized and clearly stated.

Users who frequent learn.microsoft.com often don't differentiate between various sections of the documentation. As such, I believe the same naming conventions should apply across all sections, unless otherwise specified.

(In my team, we've been using the non-underscore convention for private fields for years. With the new guidelines now available in the official documentation, we've decided to align ourselves with the mainstream and modify our practices accordingly.)

@guardrex
Copy link
Collaborator

guardrex commented Oct 1, 2023

Sure ... I understand. They may have decided to take a harder stand on it. Still tho, I'm just a small green dinosaur 🦖 in a BIG Microsoft world. Management will let me know what they'd like to do ... AND they need to take care of the Blazor project templates for the change if they go with underscores. That will require an issue over on their repo. I wouldn't necessarily open it now tho. I'm composing an internal message to them now on this, and I can report back here (or they'll pop in here and let us know) shortly. Stand-by ..................

UPDATE ... Ok ... Message sent! We should hear back soon.

UPDATE (10/2) ... Word back is that they'll discuss it and get back to us.

@danroth27
Copy link
Member

The coding guidelines referenced are actually specific to the dotnet/docs and dotnet/runtime repos as described here: https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions. While we follow many of these same guidelines in the dotnet/aspnetcore.docs repo, we diverge in some places to keep our samples and snippets consistent with the default behavior of Visual Studio and our shipped templates.

@hakenr
Copy link
Member Author

hakenr commented Oct 9, 2023

(The funny part: The https://github.com/dotnet/aspnetcore repo follows _privateField convention. Maybe its time to change the Visual Studio templates.)

@guardrex
Copy link
Collaborator

guardrex commented Oct 9, 2023

AFAIK, there's a technical reason in framework development for doing it in framework code. Ryan and Pranav (before they left) didn't elaborate, but that's what they told me. Ryan said one of my favorite PU quotes ever on this subject: He said whether or not to guide community/customer devs to _/this was ⚔️ "under civil debate" ⚔️😆. There are (or were) two camps of thought among engineers, apparently each with their favored reasoning one way or the other. At the end of the day, the NO-_ w/this camp won the battle. WRT consistency around MS in framework code and docs, it's up to this team to decide for this framework and this doc set, so here we are 😄 ... a bit of inconsistency.

I think what I'm going to do is remark on this in the Blazor section that pertains to how we present Blazor doc code to readers. I'll make a tracking note so that I don't forget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
Archived in project
Development

No branches or pull requests

3 participants