----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46191/#review129003 -----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 44) <https://reviews.apache.org/r/46191/#comment192467> How about naming this `nodename` to match the field name? 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 64) <https://reviews.apache.org/r/46191/#comment192468> How about naming this `machine` to match the field name? 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 98 - 99) <https://reviews.apache.org/r/46191/#comment192460> This is the correct format? Do we not want `major.minor.patch`? If we do want `major.minor.patch`, then we could just do `return stringify(Version(os_version.dwMajorVersion, os_version.dwMinorVersion, 0));` 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 308 - 312) <https://reviews.apache.org/r/46191/#comment192462> Can we pull this out to `Try<OSVERSIONINFOEX> internal::os_version();`? 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 310) <https://reviews.apache.org/r/46191/#comment192461> Please use a C++ cast. 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 326 - 330) <https://reviews.apache.org/r/46191/#comment192463> We can use `internal::os_version` here instead if we pull it out as suggested above. 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 355 - 360) <https://reviews.apache.org/r/46191/#comment192465> If we pull out `internal::os_version`, we could do less work here: ``` Try<OSVERSIONINFOEX> os_version = internal::os_version(); if (os_version.isError()) { return Error(os_version.error()); } return internal::sysname(os_version); ``` If the idea is to keep the implementation consistent with `posix/os.hpp` that sounds fine as well, in which case we should just define this in `os.hpp`. - Michael Park On April 14, 2016, 8:28 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46191/ > ----------------------------------------------------------- > > (Updated April 14, 2016, 8:28 a.m.) > > > Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, > Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun. > > > Bugs: MESOS-4470 > https://issues.apache.org/jira/browse/MESOS-4470 > > > Repository: mesos > > > Description > ------- > > Stout: Implemented `uname` on Windows. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp > c48106e5905e3be0faeba7177ef534766089faff > > Diff: https://reviews.apache.org/r/46191/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
