Re: Review Request 56364: Windows: Stout: Rewrite job object wrappers.

2017-03-29 Thread Andrew Schwartzmeyer
> On March 29, 2017, 9 p.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/windows/os.hpp > > Lines 781 (patched) > > > > > > This comment is a tiny bit too long (> 80 characters). Ah I really need to get VS Cod

Re: Review Request 56364: Windows: Stout: Rewrite job object wrappers.

2017-03-29 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/#review170447 --- Ship it! Looks great! I can fix the one little thing below.

Re: Review Request 56364: Windows: Stout: Rewrite job object wrappers.

2017-03-27 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- (Updated March 27, 2017, 10:43 p.m.) Review request for mesos, John Kordich, Jo

Re: Review Request 56364: Windows: Stout: Rewrite job object wrappers.

2017-03-27 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- (Updated March 27, 2017, 10:10 p.m.) Review request for mesos, Alex Clemmer and

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-12 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- (Updated Feb. 12, 2017, 6:29 p.m.) Review request for mesos, Alex Clemmer and J

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-08 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- (Updated Feb. 8, 2017, 9:34 p.m.) Review request for mesos, Alex Clemmer and Jo

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-08 Thread Andrew Schwartzmeyer
> On Feb. 8, 2017, 2:26 a.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/windows/os.hpp, line 729 > > > > > > I believe using `BOOL` as a boolean results in a warning. We often > > just compare it to `FALSE`

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-08 Thread Andrew Schwartzmeyer
> On Feb. 8, 2017, 2:26 a.m., Joseph Wu wrote: > > I need to check how this is used in the rest of the review chain, but... > > > > Giving the ownership of the HANDLE to the caller may require much larger > > changes in the codebase. You may notice that we simply leak some pid's in > > some p

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-08 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- (Updated Feb. 8, 2017, 9:10 p.m.) Review request for mesos, Alex Clemmer and Jo

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-07 Thread Alex Clemmer
> On Feb. 8, 2017, 2:26 a.m., Joseph Wu wrote: > > I need to check how this is used in the rest of the review chain, but... > > > > Giving the ownership of the HANDLE to the caller may require much larger > > changes in the codebase. You may notice that we simply leak some pid's in > > some p

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-07 Thread Alex Clemmer
> On Feb. 8, 2017, 2:26 a.m., Joseph Wu wrote: > > 3rdparty/stout/include/stout/windows/os.hpp, lines 718-721 > > > > > > This is a good default. But we need a way to toggle this behavior, > > such that the agent's

Re: Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-07 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/#review164619 --- I need to check how this is used in the rest of the review chain,

Review Request 56364: Windows: Stout: Rewrite Job Object wrappers.

2017-02-06 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56364/ --- Review request for mesos, Alex Clemmer and Joseph Wu. Bugs: MESOS-6892 http