----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52705/#review152196 -----------------------------------------------------------
3rdparty/stout/include/stout/wait.hpp (line 17) <https://reviews.apache.org/r/52705/#comment221053> How much work is it to do this now? If we're going to have a dedicated header, I would greatly prefer to put all the definitions in one place. 3rdparty/stout/include/stout/wait.hpp (line 25) <https://reviews.apache.org/r/52705/#comment221057> Couple of questions here: * This seems useful for POSIX platforms, but I don't really understand the implications of having this enabled on Windows. Windows will never support the signal semantics of POSIX, and in general my feeling is that I'd rather not define things on platforms that don't support those semantics, unless there is a particularly pertinent reason to. Thoughts? * In the 2 or 3 places we do use `W_EXITCODE`, rather than defining it for Windows, we simply `#ifdef` the code out. So the precedent is to just not use the macro. Is there a compelling reason to change this? Also, tiny, tiny nit question: is it style to have 2 spaces between `W_EXITCODE(ret, sig)` and `((ret) << 8 | (sig))` 3rdparty/stout/include/stout/wait.hpp (line 28) <https://reviews.apache.org/r/52705/#comment221058> I've been out of the game awhile-- are we wrapping comments at 70 characters these days? - Alex Clemmer On Oct. 10, 2016, 9:22 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52705/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2016, 9:22 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6310 > https://issues.apache.org/jira/browse/MESOS-6310 > > > Repository: mesos > > > Description > ------- > > This was motivated by the need for a default definition of > 'W_EXITCODE' (since it is not technically POSIX compliant). > > > Diffs > ----- > > 3rdparty/stout/include/stout/wait.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/52705/diff/ > > > Testing > ------- > > sudo GTEST_FILTER="*Nested*" src/mesos-tests > > > Thanks, > > Kevin Klues > >
