----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54358/#review158449 -----------------------------------------------------------
Fix it, then Ship it! The overall idea here looks good to me, it would be great if Michael could take a look at the esoteric C++ functionality used here. 3rdparty/libprocess/include/process/loop.hpp (lines 101 - 102) <https://reviews.apache.org/r/54358/#comment229224> "versus the others"? The other non-loop examples above? 3rdparty/libprocess/include/process/loop.hpp (lines 114 - 119) <https://reviews.apache.org/r/54358/#comment229231> This should be added in this patch? It's added in the previous patch. 3rdparty/libprocess/include/process/loop.hpp (lines 185 - 193) <https://reviews.apache.org/r/54358/#comment229234> Do you want to one-line these like you did for the value operators below? 3rdparty/libprocess/include/process/loop.hpp (line 205) <https://reviews.apache.org/r/54358/#comment229226> s/vaue/value/ 3rdparty/libprocess/include/process/loop.hpp (lines 333 - 340) <https://reviews.apache.org/r/54358/#comment229232> Do you want to switch on the enum directly here to avoid having CHECKs and instead just having the compilation fail if we miss a case? We could avoid isContinue and isBreak and just directly use the enum? 3rdparty/libprocess/include/process/loop.hpp (lines 344 - 348) <https://reviews.apache.org/r/54358/#comment229233> Ditto here about switch vs if/else. 3rdparty/libprocess/src/io.cpp (lines 447 - 449) <https://reviews.apache.org/r/54358/#comment229230> Hm.. I don't follow this comment, since we're copying 'data' when capturing it in the first lambda. 3rdparty/libprocess/src/tests/loop_tests.cpp (line 60) <https://reviews.apache.org/r/54358/#comment229228> Can you add a using clause for std::string? - Benjamin Mahler On Dec. 5, 2016, 4:36 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54358/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2016, 4:36 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Introduced ControlFlow for process::loop. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/loop.hpp > a78ea7da8a5a778d816a64fb46d9479cc2a4ed70 > 3rdparty/libprocess/src/http.cpp fc55bda0fae8a43794fb938718b3a720dc342473 > 3rdparty/libprocess/src/io.cpp e81f279ed4bf92f75ad2427550ca822a9b03cca5 > 3rdparty/libprocess/src/tests/loop_tests.cpp > 8435ba872e8d4505c3a9125e5a2dac1c31b9bf9a > > Diff: https://reviews.apache.org/r/54358/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
