----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47168/#review132458 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 16 - 18) <https://reviews.apache.org/r/47168/#comment196641> Can you group these includes properly please? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 16 - 23) <https://reviews.apache.org/r/47168/#comment196643> Can you group / order these includes properly please? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 25 - 26) <https://reviews.apache.org/r/47168/#comment196644> Can you add a comment explaining the purpose of these? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 30) <https://reviews.apache.org/r/47168/#comment196645> - Please use snake_case in stout. - `{` braces go on the next line. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 33 - 35) <https://reviews.apache.org/r/47168/#comment196647> Howcome you needed to wrap this with a second set of parentheses? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 35) <https://reviews.apache.org/r/47168/#comment196646> Error messages don't end with a period `.`. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 36) <https://reviews.apache.org/r/47168/#comment196648> Can you leave a new line between the log statement, and the return? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 39) <https://reviews.apache.org/r/47168/#comment196658> Why a `shared_ptr` instead of a `unique_ptr`? 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 52) <https://reviews.apache.org/r/47168/#comment196650> `{` goes on next line. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 54 - 55) <https://reviews.apache.org/r/47168/#comment196652> Missing periods. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 61) <https://reviews.apache.org/r/47168/#comment196653> Missing a `:` after `os::kill()`. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (line 62) <https://reviews.apache.org/r/47168/#comment196654> `Sginal` typo. 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 65 - 66) <https://reviews.apache.org/r/47168/#comment196655> Can you leave a new line after the `LOG(ERROR)` please? 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 21) <https://reviews.apache.org/r/47168/#comment196633> missing period `.`. P < S alphabetize 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 317) <https://reviews.apache.org/r/47168/#comment196634> Not sure why the `.` was removed here? 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (lines 321 - 323) <https://reviews.apache.org/r/47168/#comment196635> How about: ``` Linux signal flags not used in Windows. We define them per `Linux sys/signal.h` to branch properly for Windows processes' stop, resume and kill. ``` 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (lines 324 - 326) <https://reviews.apache.org/r/47168/#comment196637> Are the `Dec XX` comments correct? They look like they might be in the wrong order? 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 366) <https://reviews.apache.org/r/47168/#comment196638> indentation. 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (lines 396 - 397) <https://reviews.apache.org/r/47168/#comment196639> Please put `_S_IREAD` and `_S_IWRITE` in backticks. 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 398) <https://reviews.apache.org/r/47168/#comment196640> You added `_O_CREAT`, can you add a comment / explain why? - Joris Van Remoortere On May 10, 2016, 8:54 a.m., Daniel Pravat wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47168/ > ----------------------------------------------------------- > > (Updated May 10, 2016, 8:54 a.m.) > > > Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van > Remoortere, and Michael Park. > > > Repository: mesos > > > Description > ------- > > Windows: Implement `kill` and `killtree`. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am > 33ddb06e25920096f2d16d4f372129ee2f6a8721 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/kill.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/kill.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp > PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp > 3828c53c0dbbf370d642189998af75b9af434e9d > > Diff: https://reviews.apache.org/r/47168/diff/ > > > Testing > ------- > > Windows: build/test w/ Marathon > > > Thanks, > > Daniel Pravat > >
