----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62508/#review186870 -----------------------------------------------------------
Ship it! A couple nits, which I can fix before committing. 3rdparty/stout/include/stout/ip.hpp Line 69 (original), 67 (patched) <https://reviews.apache.org/r/62508/#comment263732> Nano-nit: Comment must start with a capital letter and end with a period. 3rdparty/stout/include/stout/windows.hpp Lines 16-21 (original), 32-38 (patched) <https://reviews.apache.org/r/62508/#comment263734> Going to remove this TODO as it does not have much to do with header ordering. (Rather, we can consider cleaning up this list of headers later, as some includes may no longer be in use.) 3rdparty/stout/include/stout/windows/ip.hpp Line 23 (original), 23-25 (patched) <https://reviews.apache.org/r/62508/#comment263735> Hmm... a different style of comment? Since I prefer the one-line variety, I'll change it :) 3rdparty/stout/include/stout/windows/mac.hpp Lines 23-24 (original), 23-26 (patched) <https://reviews.apache.org/r/62508/#comment263736> Different style _and_ double inclusion? :) 3rdparty/stout/include/stout/windows/os.hpp Lines 43-47 (patched) <https://reviews.apache.org/r/62508/#comment263739> Going to plop down a comment here: ``` // NOTE: These system headers must be included after `stout/windows.hpp` // as they may include `Windows.h`. See comments in `stout/windows.hpp` // for why this ordering is important. ``` - Joseph Wu On Sept. 22, 2017, 11:21 a.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62508/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2017, 11:21 a.m.) > > > Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff. > > > Bugs: MESOS-7993 > https://issues.apache.org/jira/browse/MESOS-7993 > > > Repository: mesos > > > Description > ------- > > This patch consolidates the inclusion of the ordering-dependent Windows > system headers `WinSock2.h`, `WS2tcpip.h`, `Windows.h`, etc. For > historical reasons, `Windows.h` will include `winsock.h`, the header for > the Windows Sockets 1.1 library, which was last supported in Windows > 2000. We use the Windows Sockets 2 library, which requires the > `WinSock2.h` header, and this header must be included before > `Windows.h`, otherwise symbol redefinition errors will occur. The > `WS2tcpip.h` header includes the extensions to Windows Sockets 2, and > any header which may include `Windows.h` must be included carefully. > > It is simplest to consolidate the inclusion of these problematic system > headers into `stout/windows.hpp`, and elsewhere include that with a > comment as to which header specifically the file is requiring. Doing so > will prevent incorrect ordering from being introduced. > > Note that the erroneous inclusion of `winsock.h` was removed. > > > Diffs > ----- > > 3rdparty/stout/include/stout/ip.hpp > a722fa47e05cf093e4ab4fed9d2824236dd5dd80 > 3rdparty/stout/include/stout/os/windows/dup.hpp > 1c9dda0ba4653f970167e959139afb851682c6f8 > 3rdparty/stout/include/stout/os/windows/fd.hpp > ae2db27154e694f319dafe2e04c01d55c42179de > 3rdparty/stout/include/stout/os/windows/sendfile.hpp > d50c89e4931b9109d7496fb4db496aea2ac7f830 > 3rdparty/stout/include/stout/os/windows/socket.hpp > 18d2ecf560933d6a86cf945460b8611581b58cbb > 3rdparty/stout/include/stout/windows.hpp > 1d865f8fd23aba0198017f0bf4be8471cfb714ed > 3rdparty/stout/include/stout/windows/ip.hpp > d7738f2fe9cb6818e5686dcb7dbb3cc73618f856 > 3rdparty/stout/include/stout/windows/mac.hpp > 3ebf4e15e81e882d52a769811757f713a8ae65df > 3rdparty/stout/include/stout/windows/net.hpp > 364509f62f365eb5c1fd3ffe9aa38d1cb677c131 > 3rdparty/stout/include/stout/windows/os.hpp > a70a61c702a422462872c0ec85f3c34e26a2e383 > > > Diff: https://reviews.apache.org/r/62508/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrew Schwartzmeyer > >
