----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23745/#review48341 -----------------------------------------------------------
Ship it! Looking forward to the next 7.5%! src/common/attributes.cpp <https://reviews.apache.org/r/23745/#comment84842> These are no longer inline, so lets kill the 'inline' keyword please. Can you do a sweep of this review and find all these too? src/common/date_utils.hpp <https://reviews.apache.org/r/23745/#comment84843> FYI, this is all dead code, so let's delete it while we're at it. src/common/date_utils.cpp <https://reviews.apache.org/r/23745/#comment84844> Dead code. src/common/date_utils.cpp <https://reviews.apache.org/r/23745/#comment84845> Dead code. src/common/date_utils.cpp <https://reviews.apache.org/r/23745/#comment84846> Dead code. src/common/protobuf_utils.hpp <https://reviews.apache.org/r/23745/#comment84847> Let's include a comment here that explains that we're using this forward declaration rather than including the header to set a documentation precedent. How about: // Forward declaration (in lieu of an include). src/common/protobuf_utils.hpp <https://reviews.apache.org/r/23745/#comment84848> Indentation looks off here. src/common/protobuf_utils.cpp <https://reviews.apache.org/r/23745/#comment84850> Another newline here please! ;-) src/common/protobuf_utils.cpp <https://reviews.apache.org/r/23745/#comment84849> These comments belong in the header, where most folks are going to read about these helpers. src/common/thread.hpp <https://reviews.apache.org/r/23745/#comment84852> I think this TODO is supposed to refer to 'start' not 'run', do you mind updating it while you're at it please? src/common/thread.hpp <https://reviews.apache.org/r/23745/#comment84851> This is an internal function, let's not expose it in the public interface. src/common/thread.cpp <https://reviews.apache.org/r/23745/#comment84853> This can now be a static function (private to this translation unit). - Benjamin Hindman On July 22, 2014, 12:22 a.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23745/ > ----------------------------------------------------------- > > (Updated July 22, 2014, 12:22 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, and Dominic Hamon. > > > Bugs: MESOS-1583 > https://issues.apache.org/jira/browse/MESOS-1583 > > > Repository: mesos-git > > > Description > ------- > > Dominic's Hamon review request: https://reviews.apache.org/r/18295 mergeable > to today's master version, this is a lightened version to make review easier. > Note I see an improvements of ~7.5% of the compile time. > > > Diffs > ----- > > include/mesos/resources.hpp 8fc347e > src/Makefile.am c216e2f > src/common/attributes.hpp e6271cd > src/common/attributes.cpp 851c92a > src/common/date_utils.hpp 523e273 > src/common/date_utils.cpp 1c7565d > src/common/factory.hpp 94056f6 > src/common/http.hpp 717bbe9 > src/common/http.cpp e81a7ad > src/common/parse.hpp 4eb5096 > src/common/protobuf_utils.hpp 0341c7f > src/common/protobuf_utils.cpp PRE-CREATION > src/common/resources.cpp d39001b > src/common/status_utils.hpp 1980551 > src/common/thread.hpp 7e48724 > src/common/thread.cpp PRE-CREATION > src/common/type_utils.hpp ee946a3 > src/common/type_utils.cpp PRE-CREATION > src/common/values.cpp 15583fd > src/master/master.cpp 896be5e > src/slave/containerizer/isolator.hpp 8c05cd4 > src/slave/slave.cpp 6f0e121 > > Diff: https://reviews.apache.org/r/23745/diff/ > > > Testing > ------- > > > Thanks, > > Isabel Jimenez > >
