Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-16 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 16, 2016, 8:37 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-16 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114874 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
> On Jan. 16, 2016, 12:13 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > lines 96-97 > > > > > > It seems like we're moving towards `snake_case`

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 16, 2016, 2:24 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
> On Jan. 13, 2016, 6:01 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 37-40 > > > > > > We typically don't use typedefs in Mesos. Can we just use > > `std::shared_

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114796 --- Ship it! 3rdparty/libprocess/3rdparty/stout/include/stout/interna

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 15, 2016, 7:21 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 15, 2016, 6:49 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Michael Park
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 104 > > > > > > This looks like it should return a `Try`? > > Alex

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
> On Jan. 15, 2016, 3:28 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 166-169 > > > > > > Is this supposed to return `int` like the ones below? or are they > > s

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
> On Jan. 15, 2016, 3:28 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 166-169 > > > > > > Is this supposed to return `int` like the ones below? or are they > > s

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Michael Park
> On Jan. 15, 2016, 3:28 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 166-169 > > > > > > Is this supposed to return `int` like the ones below? or are they > > s

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-15 Thread Alex Clemmer
> On Jan. 15, 2016, 3:28 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, lines > > 41-42 > > > > > > Add new line. The POSIX version of this code has the same pro

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114644 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > lines 95-96 > > > > > > I don't quite understand why these are `std::wstri

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 15, 2016, 1:58 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
> On Jan. 13, 2016, 6:01 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 37-40 > > > > > > We typically don't use typedefs in Mesos. Can we just use > > `std::shared_

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 67 > > > > > > This looks like it's > 80 chars long. Is there maybe a

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 108 > > > > > > Is this `UNREACHABLE` necessary? > > Alex Clemmer wrote: > It

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 126 > > > > > > I noticed the use of `WindowsError` below. Should th

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Michael Park
> On Jan. 13, 2016, 6:01 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp, lines 156-174 > > > > > > The parens around `mode` are no longer necessary. > > Alex Clemmer wrote

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 104 > > > > > > This looks like it should return a `Try`? Leaving t

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
> On Jan. 14, 2016, midnight, Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 67 > > > > > > This looks like it's > 80 chars long. Is there maybe a

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 14, 2016, 10:08 a.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-14 Thread Alex Clemmer
> On Jan. 13, 2016, 6:01 a.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am, line 43 > > > > > > Can we add a tab here? Per our Slack conversation, the length of this path exceeds th

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-13 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114299 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-13 Thread Michael Hopcroft
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114329 --- Ship it! Ship It! - Michael Hopcroft On Jan. 12, 2016, 2:03 a.m

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-12 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review114150 --- Started to look at this review. A few minor comments here, I'll loo

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 130 > > > > > > Might want to static_assert that _USE_32BIT_TIME_T is not defi

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 12, 2016, 2:03 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Jan. 11, 2016, 8:45 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 102 > > > > > > Please do not use a `default` branch when switching over enum

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 130 > > > > > > Might want to static_assert that _USE_32BIT_TIME_T is not defi

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 130 > > > > > > Might want to static_assert that _USE_32BIT_TIME_T is not defi

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Jan. 11, 2016, 8:45 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 102 > > > > > > Please do not use a `default` branch when switching over enum

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review113835 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.h

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 40 > > > > > > Might add a comment explaining that when ::stat() returns a valu

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-11 Thread Alex Clemmer
> On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp, > > line 61 > > > > > > Recommend opening a work item for Windows team to add t

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Yi Sun
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review112701 --- Ship it! Ship It! - Yi Sun On Jan. 5, 2016, 12:12 a.m., Alex Cl

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 5, 2016, 12:12 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
> On Jan. 4, 2016, 11:30 p.m., Yi Sun wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 86 > > > > > > We are not calling lstat here. Great catch. This should also not overwri

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Yi Sun
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review112686 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 4, 2016, 11:26 p.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review112685 --- Ship it! Ship It! - Daniel Pravat On Jan. 4, 2016, 11:46 a.m.,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Daniel Pravat
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review112661 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 154 > > > > > > Have you checked all of the return types to ensure they are wh

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 4, 2016, 11:46 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 199 > > > > > > Recommend using RAII pattern (e.g. std::unique_p

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
> On Nov. 4, 2015, 6:08 p.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp, > > line 32 > > > > > > I wonder if there is any good way of protecting ou

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 4, 2016, 11:33 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-04 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Jan. 4, 2016, 11:19 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2016-01-03 Thread Alex Clemmer
> On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/symlink.hpp, > > line 14 > > > > > > Recommend #pragma once. This is supported by VS. GCC su

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-17 Thread Alex Clemmer
> On Nov. 2, 2015, 10:57 p.m., Joseph Wu wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 146 > > > > > > I think we still need an `rdev` function. (Although it'll have to b

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-17 Thread Benjamin Bannier
> On Nov. 4, 2015, 1:02 a.m., Michael Hopcroft wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp, line > > 107 > > > > > > Is the enum+switch+unreachable a common pattern in stout? It s

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-16 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Nov. 16, 2015, 9:14 a.m.) Review request for mesos, Artem Harutyunyan,

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-04 Thread Joseph Wu
> On Nov. 3, 2015, 5:02 p.m., Michael Hopcroft wrote: > > This is my initial set of comments. I still need to read from line 112 on > > in stat.hpp and I haven't started reading reparsepoint.hpp. Replied to a few project-specific (we might call it "tribal knowledge") points you raised. If you

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-04 Thread Michael Hopcroft
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review105083 --- 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-03 Thread Michael Hopcroft
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review105007 --- This is my initial set of comments. I still need to read from line

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-11-02 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/#review104810 --- Partial review. (Haven't looked at the new files yet). `stat.hpp`

Re: Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-10-31 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- (Updated Nov. 1, 2015, 1:31 a.m.) Review request for mesos, Artem Harutyunyan,

Review Request 39803: Windows: Implemented stout/os/stat.hpp`.

2015-10-31 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39803/ --- Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu