----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13520/#review25106 -----------------------------------------------------------
Ship it! src/slave/status_update_manager.hpp <https://reviews.apache.org/r/13520/#comment49322> Well the point of annotations is for tools like doxygen, not humans, but it looks like doxygen understands multi-line @return after all! http://stackoverflow.com/questions/3436849/doxygen-javadoc-style-tag-description-spanning-multiple-lines More importantly, can you also document what this function does? Cgroups used True, False, Error rather than using strings 'true', 'false', 'Error'. Which do you prefer? I think we should not decide on the documentation style using individual opinions as it leads to inconsistent comments :(. For example, this file itself has inconsistent comment style and it was written only by you. ;) To be clear, I'm ok with doxygen style comments, I would just like to make sure we all agree on what style to use in Mesos (given it's not mentioned in the style guide). Ship It, but perhaps we should start a conversation on the @dev list if you'd like to continue using them? I'm a pedant, please forgive me. :) src/slave/status_update_manager.hpp <https://reviews.apache.org/r/13520/#comment49323> Can you comment what this function does? Also use True / False, Error akin to cgroups.hpp? src/slave/status_update_manager.hpp <https://reviews.apache.org/r/13520/#comment49327> So is this a duplicate? If not can you update the the documentation to reflect this false case? Drop this if it's a duplicate. - Ben Mahler On Aug. 13, 2013, 10:49 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13520/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2013, 10:49 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-640 > https://issues.apache.org/jira/browse/MESOS-640 > > > Repository: mesos-git > > > Description > ------- > > Refactored the StatusUpdateStream API a bit to make it easy to distinguish > duplicates from errors. > > > Diffs > ----- > > src/slave/status_update_manager.hpp > da927606dea63b62d99baac529a3fe371f2767ba > src/slave/status_update_manager.cpp > ffd47369e49f711f984787cd3710f7b4dd5b29ce > src/tests/status_update_manager_tests.cpp > 6473478a283d19f991143710bcf551e298bd6916 > > Diff: https://reviews.apache.org/r/13520/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
