I'm bewildered by this reply, seems my comments were misinterpreted? I'm suggesting that we _do_ add doxygen comments to our libraries (stout, libprocess, state, cgroups, etc) as that is a nice way to make them accessible. I'm less convinced that there's value in adding doxygen in _every_ internal header in mesos (e.g. master / slave, as these are not libraries).
Also, folks _are_ free to add documentation no matter what, I'm just suggesting that they keep the style consistent within a library header. They are free to do a conversion sweep before _or_ after, it does not block their ability to contribute to documentation, just means they need to maintain consistent formatting. Sad to see implications that I do not care about productivity, or that I'm trying to prevent people from contributing documentation... :( On Tue, Jul 14, 2015 at 8:12 AM, Marco Massenzio <[email protected]> wrote: > On Mon, Jul 13, 2015 at 9:41 PM, Benjamin Mahler < > [email protected]> > 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 <[email protected]> > > wrote: > > > > > On Thu, Jul 9, 2015 at 5:23 PM, Benjamin Mahler < > > [email protected] > > > > > > > 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 :) > > > > > >
