----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58030/#review170582 -----------------------------------------------------------
Fix it, then Ship it! src/checks/checker.hpp Line 25 (original), 25-26 (patched) <https://reviews.apache.org/r/58030/#comment243443> Swap these two. src/checks/checker.cpp Lines 165 (patched) <https://reviews.apache.org/r/58030/#comment243444> Do you need to explicitly say `process::` and `std::`? Here and in other places in this file. src/checks/checker.cpp Lines 600-616 (patched) <https://reviews.apache.org/r/58030/#comment243450> See my comment in https://reviews.apache.org/r/57854/ regarding aliases vs. `this`. src/checks/checker.cpp Lines 738 (patched) <https://reviews.apache.org/r/58030/#comment243454> Double space : ) src/checks/checker.cpp Lines 489-507 (original), 877-904 (patched) <https://reviews.apache.org/r/58030/#comment243455> I'd say having a single `processCheckResult()` call will simplify reading the code. I'd suggest we create an optional `CheckStatusInfo` at the beginning and "return" it once at the end. src/checks/checker.cpp Lines 888-892 (patched) <https://reviews.apache.org/r/58030/#comment243456> Let's `LOG()` it. src/tests/check_tests.cpp Lines 866-873 (original), 879-886 (patched) <https://reviews.apache.org/r/58030/#comment243458> We don't need this blob either. src/tests/check_tests.cpp Lines 1630 (patched) <https://reviews.apache.org/r/58030/#comment243462> I think we should increase intervals here and for the health check to avoid a new check update coming before reconciliation. - Alexander Rukletsov On March 29, 2017, 4:39 p.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58030/ > ----------------------------------------------------------- > > (Updated March 29, 2017, 4:39 p.m.) > > > Review request for mesos, Alexander Rukletsov and Vinod Kone. > > > Bugs: MESOS-7277 > https://issues.apache.org/jira/browse/MESOS-7277 > > > Repository: mesos > > > Description > ------- > > Added support for COMMAND checks to the default executor. > > > Diffs > ----- > > src/checks/checker.hpp e8af3160c9fd52ec20acf41b86bade50f4539fb1 > src/checks/checker.cpp 314354cc374b453ec12e25e3d4730a92697468cf > src/launcher/default_executor.cpp 606fd9c164052fe8d168969bbb05f6ec99180d3b > src/tests/check_tests.cpp 16f1c07e109e24d475ad593ef1992dfb9f482ba6 > > > Diff: https://reviews.apache.org/r/58030/diff/1/ > > > Testing > ------- > > `make check` under Fedora 24 and macOS including the 4 new tests > > > Thanks, > > Gastón Kleiman > >
