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 ---

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

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 > >

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

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

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 > >

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 > >

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

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!

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 > >

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

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

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

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

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

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: >

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

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

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

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

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

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 ---

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

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 > >

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 ---

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

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

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

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

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

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

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

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 ---

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

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

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

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.

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

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 ---

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 ---

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

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 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

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

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

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

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

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

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 ---

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).

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