On Mon, Jul 13, 2015 at 9:41 PM, Benjamin Mahler <benjamin.mah...@gmail.com> wrote:
> Let me try to contain the length of this thread, two points don't seem to > agree my request and benh's reply. > > (1) You're saying all non-trivial classes / methods in headers should have > javadoc, whereas benh is saying APIs. Are these the same? I'd much rather > see this focused on APIs (i.e. libraries) rather than internal > implementations (e.g. master / slave) since people operating within the > latter ideally should be comfortable reading the code. Library users, less > so. > I find difficult to follow the reasoning here: are you suggesting that every time a developer uses a "library" function they are supposed to reverse engineer the code? that feels not a very efficient way of running a large development team and it certainly was not the way we rolled at Google :) On the contrary, due to their frequent and extensive use, IMO library methods/classes ought to have _extensive_ documentation. Then again, maybe it's just me caring about productivity in my teams... > > (2) Doing the incremental change will make things inconsistent :( Given > that doing a javadoc conversion sweep for a library header is not that > tedious, it seems wise to just have consistency as a forcing function for > folks to do sweeps. Plus we keep the documentation consistently formatted, > which seems like a big win! > Sure - and +100 to that! But, in the meantime, let's not have folks *not* add javadoc (or worse, demand they remove those they may have already added during code review!) or require them to do a "sweep" only because they added ONE method and want to document that. Again, I'm trying here to lower the bar for participation for folks who are not (yet) deep experts in Mesos' internals and encourage participation of people who are excited about contributing functionality to Mesos: if the cost is to have to "reverse engineer"[*] some obscure parts of libprocess and spend days (or weeks) trying to figure out how to correctly use the base libraries, I think we'll all lose in the long run. Bottom line, Ben - if you don't feel like adding documentation/javadoc to the methods/classes you contribute, I guess that's fine: but, please, let's not prevent folks from doing so, that's all I'm saying. Thanks! [*] NOTE - I still expect people to intimately understand the functionality of libprocess/stout and whatever else is already in Mesos proper: however, that would ideally be gained by studying extensive documentation; looking at existing and sample code: and experimenting with it. What I do object to is the extra layer of effort in having to reverse-engineer large, undocumented and complex areas of the code. > > > On Fri, Jul 10, 2015 at 9:39 AM, Marco Massenzio <ma...@mesosphere.io> > wrote: > > > 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 :) > > >
