----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33823/#review83125 -----------------------------------------------------------
Ship it! Only minor nits from my side, it otherwise LGTM include/mesos/executor/executor.hpp <https://reviews.apache.org/r/33823/#comment133994> please avoid SCREAMING COMMENTS :) (also, not sure what is the use of that comment? is it a TODO? should we have a #pragma? what?) include/mesos/executor/executor.proto <https://reviews.apache.org/r/33823/#comment133992> can we add also a link to the HTTP API design doc? this way for anyone looking at the Javadoc, they can follow the link and make some sense of the generated classes. Also, as a general comment, I'd really like all `message`s to have at least a brief description, so that the Javadoc that gets generated is meaningful to folks writing clients. As a general rule, we should not expect folks who write against an API to have to read the code (they may not even be able to easily access it) and, in particular those writing Java are likely to use an IDE that will present the javadoc in a pop-up/tooltip upon auto-completion. include/mesos/executor/executor.proto <https://reviews.apache.org/r/33823/#comment133993> `message` is becoming a truly overloaded term here: as a Protobuf message, the Message class, this (string) variable here, the (Message) `message` field below... how about `errorMessage` instead? - Marco Massenzio On May 4, 2015, 10:21 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33823/ > ----------------------------------------------------------- > > (Updated May 4, 2015, 10:21 p.m.) > > > Review request for mesos, Isabel Jimenez, Marco Massenzio, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/executor/executor.hpp PRE-CREATION > include/mesos/executor/executor.proto PRE-CREATION > src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b > > Diff: https://reviews.apache.org/r/33823/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
