Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Alex Clemmer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/#review164304
---


Ship it!




Ship It!

- Alex Clemmer


On Feb. 6, 2017, 2:14 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54595/
> ---
> 
> (Updated Feb. 6, 2017, 2:14 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an `os::dup` abstraction in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
>   3rdparty/stout/include/stout/os.hpp 
> ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
>   3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Michael Park 


> On Feb. 5, 2017, 12:59 a.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/os/windows/dup.hpp, line 20
> > 
> >
> > Tiny nits: Should we `#include` `try.hpp` and `unreachable.hpp` also? 
> > Also, in this case, I thought standard practice was to include 
> > `stout/os/int_fd.hpp` instead of the platform-specific header? That's what 
> > we do everywhere else.

(1) Included `try.hpp` and `unreachable.hpp`.
(2) The pattern I've followed is to include `os/int_fd.hpp` if I'm actually 
using `int_fd`, and using it like an `int`.
I've included `os/windows/fd.hpp` in Widows specific files, because 
including `int_fd.hpp` and using `WindowsFD` seems wrong,
and using `int_fd` and using member functions like `type()` seem also wrong.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/#review164277
---


On Feb. 5, 2017, 6:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54595/
> ---
> 
> (Updated Feb. 5, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an `os::dup` abstraction in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
>   3rdparty/stout/include/stout/os.hpp 
> ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
>   3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Feb. 5, 2017, 6:14 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Addressed Alex's comments.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-05 Thread Alex Clemmer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/#review164277
---




3rdparty/stout/include/Makefile.am (line 68)


I think you need to fix this?



3rdparty/stout/include/stout/os/windows/dup.hpp (line 20)


Tiny nits: Should we `#include` `try.hpp` and `unreachable.hpp` also? Also, 
in this case, I thought standard practice was to include `stout/os/int_fd.hpp` 
instead of the platform-specific header? That's what we do everywhere else.



3rdparty/stout/include/stout/os/windows/dup.hpp (line 30)


Tiny nit: While this is not wrong _per se_, I think it is probably more 
correct to write `result == -1`.



3rdparty/stout/include/stout/os/windows/dup.hpp (line 37)


Tiny, tiny nit: we're prefixing other global-scope functions with `::`. 
Should this one be, too?


- Alex Clemmer


On Feb. 5, 2017, 1:38 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54595/
> ---
> 
> (Updated Feb. 5, 2017, 1:38 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an `os::dup` abstraction in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
>   3rdparty/stout/include/stout/os.hpp 
> ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
>   3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-02-04 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Feb. 4, 2017, 5:38 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2017-01-09 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Jan. 9, 2017, 6:47 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-21 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Dec. 21, 2016, 3:42 p.m.)


Review request for mesos, Daniel Pravat and Joris Van Remoortere.


Changes
---

Rebased.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp ed6fec3ac1c1f9dfb0585178401f4b552822a0a1 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-20 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Dec. 20, 2016, 2:30 p.m.)


Review request for mesos and Daniel Pravat.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 53d04a9b6c4a0d8b35d3c84ef24d619fdb8a2c82 
  3rdparty/stout/include/stout/os.hpp 63265eed28d7345dfa18ac1c5a22ef58a7902c62 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-14 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

(Updated Dec. 14, 2016, 3:52 p.m.)


Review request for mesos and Daniel Pravat.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 56c44911ff7015d5825a93ce7a898a60c8624ee7 
  3rdparty/stout/include/stout/os.hpp 63265eed28d7345dfa18ac1c5a22ef58a7902c62 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park

Re: Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-14 Thread Daniel Pravat 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/#review159209
---


Ship it!




Ship It!

- Daniel Pravat


On Dec. 13, 2016, 9:02 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54595/
> ---
> 
> (Updated Dec. 13, 2016, 9:02 p.m.)
> 
> 
> Review request for mesos and Daniel Pravat.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an `os::dup` abstraction in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am fc020bbf65030eb69a42d5c402e38ecb145f4c0f 
>   3rdparty/stout/include/stout/os.hpp 
> cdcbbf488d110e33e796fb0b7414793391761abf 
>   3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 54595: Introduced an `os::dup` abstraction in stout.

2016-12-13 Thread Michael Park 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54595/
---

Review request for mesos and Daniel Pravat.


Repository: mesos


Description
---

Introduced an `os::dup` abstraction in stout.


Diffs
-

  3rdparty/stout/include/Makefile.am fc020bbf65030eb69a42d5c402e38ecb145f4c0f 
  3rdparty/stout/include/stout/os.hpp cdcbbf488d110e33e796fb0b7414793391761abf 
  3rdparty/stout/include/stout/os/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/dup.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/dup.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/54595/diff/


Testing
---


Thanks,

Michael Park