Best practices for XML doc comments

[jnm2]

(Edit: I've replaced 'coding standards' with 'best practices' because I meant something very different by it than the way it is intended to be used in the context of https://github.com/nunit/docs/wiki/Coding-Standards.)

I've noticed our XML documentation is a little inconsistent. Some sentences have missing periods and some single nouns with an adjective or two do have periods. Leaving periods off of sentences is, as a starting point, neither generally a professional style nor a standard in the .NET BCL.

I'd vote for requiring the period no matter the length of the description and in scenarios where it looks awkward, rather than dropping the period, add a couple more words to make a sentence.

I think this is one of the most important coding standards best practices to have. It's user-facing and extremely visible. It reflects on the quality of NUnit as a whole. Whatever we settle on, could we put it in https://github.com/nunit/docs/wiki/Coding-Standards a coding best practices doc? I can do that and a pass over the codebase.

/cc @nunit/framework-team

---

My bucket list for other XML documentation coding standards best practices (draft):

• Consider <paramref name="parameterName"/>, <see cref="TypeOrMember"/> and sometimes even <see langword="null"/> when possible.
  This creates links in the Object Browser and intellisense and colorizes intellisense. The compiler checks the validity of the names and overloads and you can refactor with confidence.

• Add a <summary> tag for each type and member. However, prefer to give each type and member a really communicative name. If those names end up covering all the user could need to know, remove the <summary> tag to avoid super redundant documentation. Otherwise, add relevant details.

• Add a <param> tag for each parameter describing what effect it has (rather than what it is). However, prefer to give each parameter a really communicative name. If those names end up covering all the user could need to know about all the parameters, remove all the <param> tags to avoid super redundant documentation. Otherwise, add relevant details to each <param> to the extent possible.

• Don't spend any time on the <remarks> or <returns> tags since the contents are not typically seen. Important details should all be in the <summary>. If the IDE auto-inserts them, just remove them.

• Don't leave any empty tags. Either remove them or fill them out. This includes the compiler's all-or-none <param> tag enforcement.

• Consider documenting thrown exceptions with the <exception> tag.
  This can really get the consumer up to speed on things that aren't immediately obvious from the method signature.
  Sadly intellisense will only show the exception types thrown, not the message, so for important exceptions it's good to also include it in the summary:

  /// <summary>
  /// Does foo. If <see cref="OtherProperty"/> is not set, throws <see cref="InvalidOperationException"/>.
  /// </summary>
  /// <exception cref="InvalidOperationException">Thrown when <see cref="OtherProperty"/> is not set.</exception>

[ChrisMaddock]

Sounds good - all for setting up a standard. There's a fair bit of incorrect xmldoc which could also do with fixing. (Wrong params etc.)

I presume you plan to restrict all the above to just public facing types?

[jnm2]

@ChrisMaddock

I presume you plan to restrict all the above to just public facing types?

Good point. That's the motivation and primary urgency, although I do think that as we write code we should at least consider following it for internal types too on a case by case basis. It would sure help when getting up to speed reviewing PRs. Not sure what others think.

[CharliePoole]

I would vote against adding much more to our coding standards. Coding standards should be minimal and only include those things that we are willing to impose on everyone. If we are specifying periods, then I think we are getting overly detailed.

I often tell teams to think of the coding standard as if it were the boss - that is, would you like it if your boss were continually telling you to do this thing. In a self-driven team, the coding standard is, of course, not a boss. The team is the boss and nothing should be in the coding standard unless the team actually agrees with it. Don't put stuff in and say we can make an exception - just put in stuff for which you won't make any exceptions.

This all comes from a particular point of view, of course. Forty years in the industry with the last 15 as a coach, working with teams to set up XP practices, including coding standards. I never found a coding standard in the wild that couldn't be shortened. They almost always include things that nobody really cares enough about. Make the coding standard about how to write minimally acceptable code, not perfect code. IOW, if you put periods in the coding standard, then that should mean you plan to reject PRs for want of a period and that seems silly to me.

[CharliePoole]

@jnm2 You have made this apply to the framework only by virtue of putting it here. Is that your intent?

Further, regarding XML comments, they are not required on public-facing types at all unless we are talking about an assembly that requres them. Not all of our assemblies do.

[jnm2]

Yes, I think the polish on NUnit's public-facing types is important enough that I would request changes on any PR that does not include a period, as I have been doing. That's the minimal standard that I personally consider as important for professionalism as requesting that a typo be fixed.

And yes, framework only. The framework is the only place where end users see XML documentation, right?

[CharliePoole]

@jnm2 I guess I might have said first that I do like all your points if you are talking about a guide for how to write good XML comments. But you said coding standard, which is a set of mandatory requirements, and that's what I was focusing on.

[CharliePoole]

@jnm2 OK, just checking about the framework.

Here's my take on what a coding standard means...

If you want a period on the end of every sentence, you can request it whether the coding standard says it or not. That's how we do almost everything. Programmers can take your advice or not, but mostly take it. That's the level of authority I had as a Coach for fifteen years. People did what I suggested because they respected my opinion. People will do the same when you review their code, with or without a coding standard to give you authority.

OTOH, if it's in the coding standard, you or anyone else can demand that the period be added. A Coding Standard is not a bunch of suggestions or a guideline, but a set of fairly arbitrary requirements. It's particularly good for things that could go one way or another, but for which we want to standardize, like naming conventions. That's why I say that a Coding Standard should be minimal in any self-directing team. Obviously, this is totally different from any corporate standards that are handed down from above.

[jnm2]

To clarify where I'm coming from, good public XML documentation and exception messages without misinformation, poor grammar or misspellings is no less important from my perspective than good implementation without bugs or undue sloppiness. Just like I wouldn't consider it an okay state of things to have failing tests and buggy code in master, I wouldn't consider it an okay state of things to have poor XML documentation.

I generally agree with you about coding standards. It's a really good point that I like so much that I keep hoping for a conversation to come up where I can agree with you about its application! 😃 This is making me realize that XML documentation (and error messages) are unique from all other coding standards. It's almost a separate product from the implementation. I'd personally be happy to enforce an invariant to achieve the goal of professional user-facing XML documentation in exactly the same manner that we enforce invariants of testing on the implementation.

[jnm2]

But that's just my vote. I'm interested in your and the rest of the team's reaction to the idea. Do you have a vision of some kind of long-term bar to be set for XML documentation quality? How high would the bar be? Is it better to stay as we are with no long-term bar?

[jnm2]

I guess I might have said first that I do like all your points if you are talking about a guide for how to write good XML comments. But you said coding standard, which is a set of mandatory requirements, and that's what I was focusing on.

One more factor. Contributors go looking for standards to follow. I do it, I see evidence of others doing it. They usually look at the coding standard and at the nearby files to decide how to proceed. We should have some way to document recommendations to follow at the contributor's discretion even if we do not wish to make them a coding standard.

[CharliePoole]

@jnm2

"good public XML documentation and exception messages without misinformation, poor grammar or misspellings is no less important from my perspective than good implementation without bugs or undue sloppiness. Just like I wouldn't consider it an okay state of things to have failing tests and buggy code in master, I wouldn't consider it an okay state of things to have poor XML documentation."

<chiding>
I'm sorry, but I have to call you on that one. It's a fallacy to assume that anyone who disagrees with you must not share your values of good workmanship. That, in fact, is the entire point I'm trying to make about standards. If one writes a standard that says "documents must have properly spelled words" for example, one is not only stating the obvious but insulting one's fellow team members. That is exactly the sort of standard I have found in many corps, where a separate organization created policies and procedures for programmers to follow. Just as we do not write a standard that says "C# programs must be properly written and follow good principles of organization" we don't want one that says the same about XML documentation.

Our disagreement is not about documentation or it's value or about workmanship in general. It's about the nature and purpose of a coding standard.

To suggest that one who disagrees with you values good workmanship less than you do is an ad hominem argument of the worst sort.
</chiding>

Sorry 'bout that. 😄

The term "Coding Standard" (in caps) is a term of art in most agile and definitely in XP development. It does not mean any of the things I'm seeing suggested as the purpose of this coding standard. The Coding Standard we have was written, principally by me, using the XP sense of the term. You guys could change that, but since there are lots of other words, I hope you won't.

What you describe is (to my mind) more like "Best Practices in XML Documentation" whereas a Coding Standard is (in my definition) more like "Minimal Practices." It wouldn't matter except that I think we actually need something that describes a bare minimum a lot of the time.

For example, much of what is done on NUnit is done by casual contributors. Now, if we send people away when they have not followed all your best practices, then they may not come back and we will have fewer contributors. OTOH, if we accept their work and then work to improve it as well as to get them to do better next time we will have a better chance of retaining them.

Finally, I have to add this: if your ideas are so good, why do you need the force of "legal" authority behind them? 😸 Personally, I don't think you do.