On Thu, Jul 9, 2015 at 5:23 PM, Benjamin Mahler <benjamin.mah...@gmail.com> wrote:
> A couple of thoughts: > > (1) When introducing javadoc comments, can we please keep comment style > consistent within files and APIs? For the most part, it seems folks are > introducing javadoc in consistent sweeps, which is great. However, it looks > also like there are reviews and commits where we are introducing javadoc + > non-javadoc within a file / api, would love to avoid the inconsistency. :( > This is a great suggestion, and I am really excited to see people doing this and helping us having a great, well-documented codebase! Until we get to the point where the majority of the codebase is well documented, I would suggest we use what in past similar situations I called "the ratchet" concept: whatever is added must be Done Right, and you can never slip back. This will, in due course, get us all where we want to be, without slowing progress too much. (Am I correct in assuming you too were *not* suggesting that, if we add 1-2 methods with javadoc-style docs, *all* existing ones must be updated too, right?) > (2) Where are we planning to introduce javadoc comments? APIs only? All > headers? Would love to see some communication around how we'd like folks to > be proceeding. Maybe I missed it, but can't seem to find an email with > this. > The idea would be to have javadoc-style Doxygen comments for all header files, for all *non-trivial* public classes/methods - initially, this will be a *requirement* only for newly added code, with the occasional "sweep" on existing classes; hopefully, we'll eventually get to the point where the "undocumented wilderness" footprint has shrunk to the point where we can mandate complete compliance. I think it would also be great to encourage "drive-by" additions: it's often the case that one spends time trying to figure out how an (as yet, undocumented) API/method works while they are using it in other parts of the code, and it would be a shame to waste that effort. If that's done in a "chained" patch, so much the better, but the "admin" burden is sometimes not worth the effort: again, I'd like to encourage folks to add as much docs as they feel like doing, by lowering the barriers to doing so. > (3) I ask because there is a tradeoff: we make the code more verbose to > navigate visually. Also, sometimes we document things unnecessarily: > Couldn't agree more! That was the "non-trivial" part of my comment above :) > /** > * Sends a message with data without a return address. > * > * @param to Receiver of the message. > * @param name Name of the message. > * @param data Data to send (gets copied). > * @param length Length of data. > */ > void post(const UPID& to, > const std::string& name, > const char* data = NULL, > size_t length = 0); > > Here, having a 'to' or 'receiver' as a variable name is pretty > self-evident, ditto for 'messageName', 'data', 'length'. Are we ok with > omitting these kinds of comments? It seems like we have to be asking > ourselves when this provides value. Thoughts? > +1 Thanks for raising the issue, Ben - and sorry for not doing this before: I got over-enthusiastic about having great documented code :)
