Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-12-08 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/#review193291 --- Ship it! Ship It! - Andrew Schwartzmeyer On Dec. 7, 2017,

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-12-07 Thread Jeff Coffler
> On Nov. 16, 2017, 5:50 p.m., Michael Park wrote: > > 3rdparty/stout/include/stout/os/posix/copyfile.hpp > > Lines 45 (patched) > > > > > > The backslash at the end check seems a bit odd. Doesn't `isdir` return > >

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-12-07 Thread Jeff Coffler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/ --- (Updated Dec. 8, 2017, 2:41 a.m.) Review request for mesos, Andrew

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-16 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/#review191201 --- Fix it, then Ship it!

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-07 Thread Jeff Coffler
> On Nov. 3, 2017, 6:29 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/os/windows/copyfile.hpp > > Lines 56-57 (patched) > > > > > > The general pattern is to just include the reason for an error, and

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-03 Thread Andrew Schwartzmeyer
> On Nov. 3, 2017, 11:29 a.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/os/windows/copyfile.hpp > > Lines 56-57 (patched) > > > > > > The general pattern is to just include the reason for an error, and

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-03 Thread Benjamin Mahler
> On Nov. 3, 2017, 6:29 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/os/windows/copyfile.hpp > > Lines 56-57 (patched) > > > > > > The general pattern is to just include the reason for an error, and

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-03 Thread Andrew Schwartzmeyer
> On Nov. 3, 2017, 11:29 a.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/os/windows/copyfile.hpp > > Lines 56-57 (patched) > > > > > > The general pattern is to just include the reason for an error, and

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-11-03 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/#review190043 --- 3rdparty/stout/include/stout/os/windows/copyfile.hpp Lines 56-57

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-19 Thread Jeff Coffler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/ --- (Updated Oct. 19, 2017, 9:32 p.m.) Review request for mesos, Andrew

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-19 Thread Jeff Coffler
> On Oct. 17, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/os/posix/copyfile.hpp > > Lines 38 (patched) > > > > > > nit: whitespace Removed blank line. > On Oct. 17, 2017, 11:16

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-19 Thread Jeff Coffler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/ --- (Updated Oct. 19, 2017, 6:16 p.m.) Review request for mesos, Andrew

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-17 Thread Andrew Schwartzmeyer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/#review188413 --- Ship it! Looks good to me other than some funky indentation.

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-16 Thread Jeff Coffler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/ --- (Updated Oct. 17, 2017, 1:16 a.m.) Review request for mesos, Andrew

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-16 Thread Jeff Coffler
> On Oct. 8, 2017, 2:56 a.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/tests/os/copyfile_tests.cpp > > Lines 33 (patched) > > > > > > This is being repeated... and it's already part of the base class: > >

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-13 Thread Andrew Schwartzmeyer
> On Oct. 7, 2017, 7:56 p.m., Andrew Schwartzmeyer wrote: > > The commit message needs to be in the past tense, and generally you can > > ignore mentioning stout as the files imply it (though it still gets > > mentioned in commits a lot). E.g.:: > > > > > Added `os::copyfile(from, to)`. > > >

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-12 Thread Jeff Coffler
> On Oct. 8, 2017, 2:56 a.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/os/posix/copyfile.hpp > > Lines 16 (patched) > > > > > > What was this included for? My only guess is `WIFEXITED`? Removed,

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

2017-10-11 Thread Jeff Coffler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60621/ --- (Updated Oct. 11, 2017, 11:30 p.m.) Review request for mesos, Andrew