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

XML comments style guidelines #1489

Closed
JeremyKuhne opened this issue Jul 26, 2019 · 15 comments
Closed

XML comments style guidelines #1489

JeremyKuhne opened this issue Jul 26, 2019 · 15 comments
Labels
📚 documentation Documentation bug or improvement design-discussion Ongoing discussion about design without consensus

Comments

@JeremyKuhne
Copy link
Member

JeremyKuhne commented Jul 26, 2019

In CoreFX we didn't call out a standard for XML comments. Since we have so much active cleanup and code generation I think we should develop some consistency guidelines, with the following goals:

Goals (in priority order)

  1. Code style is consistent
  2. Comments are readable
  3. Comments are maintainable
  4. Comments have real value

Here are some starting guidelines for discussion:

Guideline 1: Use XML style comments for methods and classes.

// DO

/// <summary>
///  Start the timer.
/// </summary>
public void Start() {}

// DO NOT

// Start the timer.
public void Start() {}

Guideline 2: Indent tag content by one extra space (two total) when on separate lines for readability.

// PREFER

/// <summary>
///  Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut
///  labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco
///  laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in
///  voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
///  non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
/// </summary>

// AVOID

/// <summary>
/// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut
/// labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco
/// laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in
/// voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
/// non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
/// </summary>

Guideline 3: Avoid adding comments that simply repeat the name of the element.

// DO NOT

/// <summary>
///  MyClass class.
/// </summary>
public class MyClass {}

Guideline 4: Use <see cref="" /> and <paramref name="" /> blocks when refering to other code elements or URLs. This helps with code navigation and will keep comments up to date when refactoring.

// DO

/// <summary>
///  Opens a handle for the given <paramref name="device"/>.
///  Use <see cref="CloseHandle(IntPtr)" /> to close the handle
///  when no longer needed.
/// </summary>
public IntPtr OpenHandle(string device) {}

// DO NOT

/// <summary>
///  Opens a handle for the given device. Use CloseHandle()
///  to close the handle when no longer needed.
/// </summary>
public IntPtr OpenHandle(string device) {}

cc: @bartonjs

@bartonjs
Copy link
Member

Guideline 4: Use and blocks when refering to other code elements or URLs.

I'm guessing <paramref> and/or <see> were in there, but didn't get escaped? 😄

@JeremyKuhne
Copy link
Member Author

but didn't get escaped?

Heheh :) Fixing

@RussKie
Copy link
Member

RussKie commented Jul 26, 2019

In a full accord with these 💯

@RussKie RussKie added the 📚 documentation Documentation bug or improvement label Jul 26, 2019
@bartonjs
Copy link
Member

The indentation I've done is actually two spaces, the same as VS was doing for nested tags (IIRC).

If comments exist I think the guidelines here are reasonable. But I, personally, have a love/hate relationship with API comments in code... they get written in code, then they get ported to the docs site (if the member was new), then the docs site is the authority. So, personally, I'm sort of an advocate for the comments to be stripped out once they move to the docs site.

@RussKie
Copy link
Member

RussKie commented Jul 26, 2019

So, personally, I'm sort of an advocate for the comments to be stripped out once they move to the docs site.

What about the intellisense?

@JeremyKuhne
Copy link
Member Author

The indentation I've done is actually two spaces,

I clarified

@bartonjs
Copy link
Member

What about the intellisense?

From inside the assembly, or outside? From outside it's provided by bundles that are built from the information in dotnet-api-docs (the same thing that feeds docs.microsoft.com). From inside it might/might-not matter.

Most projects I've worked on don't bother xmldocs non-public members, so "internal public calls" and "internal internal calls" equally having no docs is a potentially sane state to be in.

@hughbe
Copy link
Contributor

hughbe commented Jul 29, 2019

I’m not sure about #2 - two spaces after doc comment isn’t something I’ve really ever seen

@merriemcgaw merriemcgaw added the design-discussion Ongoing discussion about design without consensus label Jul 29, 2019
@RussKie RussKie mentioned this issue Aug 8, 2019
@RussKie
Copy link
Member

RussKie commented Aug 8, 2019

I've reformatted xml-docs to have 2 spaces in #1591

@weltkante
Copy link
Contributor

Is there some editor config in place? Last I checked this is not the default of VS. Without editor support you'll have a hard time keeping this consistent.

@sharwell
Copy link
Member

sharwell commented Aug 9, 2019

Guideline 2: Indent tag content by one extra space (two total) when on separate lines for readability

This is not directly supported by the IDE formatter and will not be sustainable in a repository. I recommend removing this guideline, as it only serves to further discourage the inclusion of documentation which is already an uphill battle before this.

@sharwell
Copy link
Member

sharwell commented Aug 9, 2019

💡 If you want assistance in XML documentation syntax, consider adding DotNetAnalyzers.DocumentationAnalyzers to the project. It has a bunch of rules to help get things like paragraphs and lists correct, and even has a refactoring that allows a comment to initially be written as Markdown and then converted to XML syntax. More information about the specific rules can be found on its documentation page. If you have questions about any of the incomplete documentation pages please feel free to ask.

@sharwell
Copy link
Member

sharwell commented Aug 9, 2019

Guideline 4: Use <see cref="" /> and <paramref name="" /> blocks when refering to other code elements or URLs.

📝 This would be aided by DotNetAnalyzers/DocumentationAnalyzers#27 and DotNetAnalyzers/DocumentationAnalyzers#29. It already detects <c>paramName</c> from DotNetAnalyzers/DocumentationAnalyzers#24 and DotNetAnalyzers/DocumentationAnalyzers#15 but doesn't detect plain paramName.

@RussKie
Copy link
Member

RussKie commented Aug 9, 2019

Without editor support you'll have a hard time keeping this consistent.

This is not directly supported by the IDE formatter and will not be sustainable in a repository.

I completely understand it. We can do a cleanup/reformat pass every so often, no real issue here.

If you want assistance in XML documentation syntax, consider adding DotNetAnalyzers.DocumentationAnalyzers to the project.

Thank you, will look into this.

@merriemcgaw merriemcgaw added this to the Future milestone Oct 10, 2019
@JeremyKuhne
Copy link
Member Author

We've settled on the above as the current working set.

@RussKie RussKie removed this from the Future milestone Aug 11, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📚 documentation Documentation bug or improvement design-discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests

7 participants