Drake: Doxygen Style Guide

[liangfok]

Problem Definition

Doxygen supports numerous documentation styles. As an example, this page lists four different ways to create a documentation block. For the same reasons we adopted a C++ style guide, we need to adopt a Doxygen style guide.

I'll start conversation started by pointing to:

1. Mozilla Developer Network's Doxygen style guide — a possible model to follow.
2. https://github.com/RobotLocomotion/drake/pull/2023#issuecomment-207096198 — a possible exception to the model
3. https://github.com/sherm1/drake/blob/add-common-nicetypename/drake/common/nice_type_name.h — actual example of compact style.

Descriptions for Non-Public-API Code

Drake contains code that's not part of the public API and thus should not be commented using Doxygen, but should still be commented for developers to read. One example is method GetActuatorEffortLimit() in RigidBodyTreeURDF.cpp. We need to define a best practice on to document this code. One option is to apply the same comment style for things like parameters, exceptions, and return values, but simply prefix each line with //.

Wish List

• [ ] Update cpplint and clang-format to enforce a Doxygen style guide.

[sherm1]

Can you point to the Mozilla doxygen style guide, Liang? The link above isn't right and I had trouble Googling it.

[liangfok]

Oops, it was an omission on my part. I've updated my original post. For good measure, here is the page I was looking at: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Interface_development_guide/Commenting_IDL_for_better_documentation

[sherm1]

I agree about the utility of some doxygen conventions. The Mozilla doc looks like conventions for use by their own documentation tool for IDL that supports only a subset of doxygen commands. So I'm not sure it is a good model for us. For example, it says don't use [in][out], but that appears to be motivated by the fact that IDL has those constructs explicitly - that would be different in C++. I could be misinterpreting.

[david-german-tri]

Should we have a vote on this? If so, I think we need to collect a few options.

My submission is simple: prefix every line in a Doxygen comment with ///, and otherwise conform to the style guide section on comments as if you were using //.

[sherm1]

Block and line comments are both useful for doxygen just as they are in code. For block comments (which we definitely want to encourage, esp. for class documentation), it is both easier and more economical to use this form:

/** First line of text.
More lines of text with a full 80 chars available.
Yet more stuff. **/

Anyway I don't think it is the comment style that needs standardizing, but more technical things like whether we require a @brief, use AUTOBRIEF, or have brief be optional; should @param and @tparam be required? Should we use param [in,out]? Must there be at least some doxygen for every class in the user-facing API, and every method in such classes (I think so).

An important standard would be for the programmer to test the generated doxygen to see how it comes out -- it is very easy to have bugs that cause crappy formatting. (Could we do that automatically so reviewers could include the doxygen in their reviews?) For example, if you mention a template type in a comment the angle brackets are likely to get misinterpreted by doxygen as html (backticks help).

[jwnimmer-tri]

require a @brief, use AUTOBRIEF, or have brief be optional;

I vote for AUTOBRIEF, to minimize ceremony within the code.

should @param and @tparam be required? Should we use param [in,out]?

I vote:

• MUST use when their absence leads to confusion.
• MUST NOT use whey they are content-less (@param foo the foo to use).
• Otherwise, author's discretion.

Must there be at least some doxygen for every class in the user-facing API, and every method in such classes (I think so).

I vote:

• MUST have doxygen class overview for every non-private class.
• MUST have doxygen on methods when its absence leads to confusion.
• Otherwise, author's discretion.

(Could we ... include the doxygen [output in] reviews?)

When #1811 is finished, we will have doxygen run as part of PRs. Hopefully we could make an easy way to click through to it while reviewing the PR.

[jwnimmer-tri]

We had a group meeting today that proposed the following conclusions:

• Doxygen HTML output is a first-class output of our project.
  
  
  
  • Because not all of our users are good enough programmers to learn from the code directly.
  
  
• Our coding standard requirement for doxygen is that comments that look okay in C++ must also look okay in the HTML -- e.g., bullet lists cannot be mangled.
  
  
  
  • Because otherwise it will be misleading in HTML output.
  
  
• Is not an a-priori defect for an author to avoid using Doxygen’s fancy features. Only if their use is inconsistent or confusing is it a defect.
  
  
  
  • We don’t want to waste time.
  
  
• The mixed-///-and/*/ on a single target (one method or class or …) is disallowed.
  
  
  
  • This is awful and everyone hates this.
  
  
• Never use backslash (e.g. Qt style \param) doxygen magic; use at (e.g. @param) instead.
  
  
  
  • Because backslash is used for many other things in C++ such as macro continuations.
  
  

[sherm1]

The mixed-///-and/*/ on a single target (one method or class or …) is disallowed.

To clarify, we didn't prohibit mixing these styles in general, just the particular use

/// Brief goes here                                          // <-- don't do this
/** Main doxygen comment is here. **/

Rather, use

/** Brief goes here. Main doxygen comment is here. **/
or                                                          // <-- these are both OK
/// Brief goes here. Main doxygen comment is here.

[sherm1]

Doxygen supports Markdown syntax (so most of your GitHub commenting skills are transferable). One thing I've found particularly useful is backticks for emphasizing that a name is actually code. For mentioning parameter names in a comment, if you have been using

@p param_name

(which I find makes the comment ugly to read in the header), you might consider

`param_name`

instead (doxygen does nothing with the @p other than change font). Per today's discussion, is also acceptable not to get fancy with parameter names at all.

[jwnimmer-tri]

Are we doing with without this? Can we close the issue?

[liangfok]

Yeah, looks like we've settled on "author's discretion" + "be consistent with the convention employed in the same file".