> On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/include/process/async.hpp, line 123 > > <https://reviews.apache.org/r/26246/diff/1/?file=710330#file710330line123> > > > > we prefer // comments
There were 2 versions of a function with this line, except one had a `//` comment and `/* */` comment. It just happened so that the one I kept had the `/* */` comment. Changed to `//` comment. > On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/include/process/async.hpp, line 204 > > <https://reviews.apache.org/r/26246/diff/1/?file=710330#file710330line204> > > > > std::move should be added to configure.ac I'll work on fixing this in a separate patch. > On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/include/process/c++11/deferred.hpp, line 278 > > <https://reviews.apache.org/r/26246/diff/1/?file=710332#file710332line278> > > > > why did you move these? having friends at the top of private: is > > expected. I didn't know, moved back to the top. It's not specified in the style guide, and the Google style guide just says: "Friend declarations should always be in the private section. If copying and assignment are disabled with a macro such as DISALLOW_COPY_AND_ASSIGN, it should be at the end of the private: section, and should be the last thing in the class." > On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/include/process/c++11/dispatch.hpp, line 265 > > <https://reviews.apache.org/r/26246/diff/1/?file=710334#file710334line265> > > > > this comment isn't accurate any more :D Removed. > On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/include/process/future.hpp, line 345 > > <https://reviews.apache.org/r/26246/diff/1/?file=710336#file710336line345> > > > > i think you'll need the line length NOLINT here. did you run the > > checkstyle? I do indeed need `NOLINT` here. I've added it back to keep consistent with others, but I don't see why we would use it here. Isn't it better to just wrap it properly? I think the only reasonable use case for `NOLINT` was for the preprocessor expansions, since the expanded source code resulted in one long line. > On Oct. 1, 2014, 9:26 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/src/tests/process_tests.cpp, line 850 > > <https://reviews.apache.org/r/26246/diff/1/?file=710340#file710340line850> > > > > addAndRun might be more descriptive Reverted to just `run`. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26246/#review55133 ----------------------------------------------------------- On Oct. 5, 2014, 5:23 a.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26246/ > ----------------------------------------------------------- > > (Updated Oct. 5, 2014, 5:23 a.m.) > > > Review request for mesos and Dominic Hamon. > > > Repository: mesos-git > > > Description > ------- > > With variadic template support now required in Mesos, the preprocessor > expansion technique is no longer necessary. > For `defer.hpp`, `deferred.hpp`, `delay.hpp`, `dispatch.hpp` and > `executor.hpp`, there are 2 separate versions. > I've removed the usage of `<stout/preprocessor.hpp>` from the `c++11/` > directory but left the pre-c++11 versions alone. > There are implementation details that make the pre-c++11 versions more tricky > to fix and I believe it's better to just let them obselete out. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/async.hpp > 9af3cc07334eb7dc26b73964c447cc9b9799396c > 3rdparty/libprocess/include/process/c++11/defer.hpp > 7b4dd07696748f2e2e20add62ecd554da516ae8f > 3rdparty/libprocess/include/process/c++11/deferred.hpp > 352205b835bd969d9560f2319e8e9d2b2f14d17b > 3rdparty/libprocess/include/process/c++11/delay.hpp > 5f686db1df50829a5aad76eb91ea6a86e8969c1d > 3rdparty/libprocess/include/process/c++11/dispatch.hpp > 76da2828cf36b6143d627dac66f3e0cc4416bae4 > 3rdparty/libprocess/include/process/c++11/executor.hpp > 157a1d29fa6e64f823b0ab40ec51889bf8947f60 > 3rdparty/libprocess/include/process/future.hpp > 46ae16b0bbce79005f5ed8711be663eeeb8f4bcf > 3rdparty/libprocess/include/process/help.hpp > 07e99f1124c60003f4c8b8e6cdcb193bb3577496 > 3rdparty/libprocess/include/process/run.hpp > 924d31a7d1bba0a6ecd8be8d622a3ae1faebe042 > 3rdparty/libprocess/src/help.cpp 85e1bdec8d7e8f46477d0f3d88847baeca2dcc9c > 3rdparty/libprocess/src/tests/process_tests.cpp > b985fb77ea05fae5c0b144ea48814acc7bb5135b > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > c2c9a5e47d37b5a3ac4b3326bde0548b5d0cbb29 > > Diff: https://reviews.apache.org/r/26246/diff/ > > > Testing > ------- > > `make && make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, > `clang-3.3` and `clang-3.5` > > > Thanks, > > Michael Park > >
