Docblocks and comments

[jcarouth]

There have been conversations going on regarding docblocks and usage within projects at Liftopia. The place to have those conversations is here. We have a style guide adopted by the team which does apply to ruby projects including rtopia and all Rails projects. We should discuss the relevance of the styleguide and propose/make changes as necessary to reflect the current opinions of the team now.

The relevant section says:

Function comments

Every function declaration should have comments immediately preceding it that describe what the function does and how to use it. These comments should be descriptive ("Opens the file") rather than imperative ("Open the file"); the comment describes the function, it does not tell the function what to do. In general, these comments do not describe how the function performs its task. Instead, that should be left to comments interspersed in the function's code.

Every function should mention what the inputs and outputs are, unless it meets all of the following criteria:

• not externally visible
• very short
• obvious

In Ruby, two popular function documentation schemes are TomDoc and YARD. You should favor using YARD over TomDoc. TomDoc is allowable since there are existing modules using it, but all new documentation should use YARD format.

RFC @liftopia/engineering

[jakefolio]

I think the places where docblocks aren't necessary or serve the least value is: specs, migrations, or generic controller methods [:index, :show, :create, :edit, :update, :destroy].

[jcarouth]

My preference is to document the public API of classes with docblocks. The internals of the class implementation (private methods, etc) are a nice-to-have with regards to docblocks but generally not necessary. I feel like my preference aligns with the styleguide already because most internal methods will be short and obvious by naming convention. The only real benefit to having docblocks on them would be to indicate the return types and make it obvious when and what an invocation will raise but if you're adhering to keeping the method simple and small that isn't that big of a deal to me.

I don't find value in documenting most controller methods (similar to @jakefolio's statement above.) I do think documenting helpers and such can be valuable as they are intended to be consumed and used.

It's also not necessary to have documentation for methods like def exists? or def inactive? as it's extremely obvious what those methods do and how to use them. The only problem is "obvious" is very hard to define which is why I think style guides like this have a broad perspective.

[jimnist]

i would modify the text above to say 'new documentation should be done using YARD' and not even mention TomDoc.

i'm fine with saying helpers that are not obvious should be commented, but would rather steer clear of "all methods of X type need comments" and also think that files/module/class documentation that is not YARD can be actually more useful in the code than docblocks.

external APIs need super clear documentation other than that, the code should be mostly self documenting, but when it's not there should be comments. realize that "is self documenting" and "obvious" are subjective.

[chou]

I think docblocks provide no marginal value at high marginal cost.

Value: none. They repeat information available in method test, method implementation, and method usage.

• If there are comprehensive tests accompanying the code, the use cases are clearly delineated.
• If there is good implementation, the code is self-documenting. All the things declared by docblocks (in static text that needs to be updated with all code changes) are immediately obvious.
• If you do not violate Liskov, it is impossible for there to exist use cases outside of what has already been doubly demonstrated through usage and tests.

Cost: Present time cost incurred writing them. This cost extends into perpetuity in the form of maintenance.

For example, a ton of our existing docblocks claim "returns nothing", which is false because it is literally impossible in Ruby. Do we plan to go update all of them so that they are truthful?

[jcarouth]

"Returns nothing" is a Tomdoc convention. http://tomdoc.org/