Hey Ben, my apologies - I expressed myself incorrectly: what I meant was a generic "you random guy" (which I know, the correct way would have been to use "one" and I didn't, my bad). I promise never again to send an email at 8am, sleep-deprived and oxygen-deprived in a crowded train.
*Marco Massenzio* *Distributed Systems Engineer* On Tue, Jul 14, 2015 at 1:54 PM, Benjamin Mahler <[email protected]> wrote: > 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 :) > > > > > > > > > >
