Re: Review Request 67388: Moved: `io::internal::read/write` to separate file.

2018-06-08 Thread Andrew Schwartzmeyer


> On June 4, 2018, 4:53 p.m., Joseph Wu wrote:
> > General note on code organization:
> > 
> > Since you are splitting the `io::internal::read/write` functions into Posix 
> > and Windows implementations, it would be helpful to organize the code like:
> > ```
> > 3rdparty/libprocess/src/
> > |-- io/
> > |-- io.cpp
> > |
> > |-- posix/
> > |   |-- io.hpp // Instead of io_internal.hpp
> > |   |-- io.cpp // Instead of poll_io.cpp
> > |
> > |-- windows/
> > |   // Instead of libwinio_impl.hpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio.hpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.hpp 
> > |
> > |   // Instead of libwinio_impl.cpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio_eventloop.cpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.cpp 
> > |
> > |-- io.hpp 
> > |-- io.cpp // Instead of libwinio_io.cpp ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- ...
> > ```

+1


- Andrew


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


On May 30, 2018, 11:55 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated May 30, 2018, 11:55 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The implementation in `io::internal::read/write` uses `io::poll`, which
> is UNIX specific. The Windows IOCP implementation will not use a
> polling function, since there no unified mechanism, so the functions
> have been moved to their own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
>   3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 
>   3rdparty/libprocess/src/poll_io.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67388/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>



Re: Review Request 67388: Moved: `io::internal::read/write` to separate file.

2018-06-04 Thread Joseph Wu

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



General note on code organization:

Since you are splitting the `io::internal::read/write` functions into Posix and 
Windows implementations, it would be helpful to organize the code like:
```
3rdparty/libprocess/src/
|-- io/
|-- io.cpp
|
|-- posix/
|   |-- io.hpp // Instead of io_internal.hpp
|   |-- io.cpp // Instead of poll_io.cpp
|
|-- windows/
|   // Instead of libwinio_impl.hpp ( 
https://reviews.apache.org/r/67389/ )
|   // Possibly combine libwinio.hpp too ( 
https://reviews.apache.org/r/67390/ )
|-- event_loop.hpp 
|
|   // Instead of libwinio_impl.cpp ( 
https://reviews.apache.org/r/67389/ )
|   // Possibly combine libwinio_eventloop.cpp too ( 
https://reviews.apache.org/r/67390/ )
|-- event_loop.cpp 
|
|-- io.hpp 
|-- io.cpp // Instead of libwinio_io.cpp ( 
https://reviews.apache.org/r/67390/ )
|-- ...
```

- Joseph Wu


On May 30, 2018, 11:55 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated May 30, 2018, 11:55 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The implementation in `io::internal::read/write` uses `io::poll`, which
> is UNIX specific. The Windows IOCP implementation will not use a
> polling function, since there no unified mechanism, so the functions
> have been moved to their own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
>   3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 
>   3rdparty/libprocess/src/poll_io.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67388/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>



Review Request 67388: Moved: `io::internal::read/write` to separate file.

2018-05-30 Thread Akash Gupta

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
John Kordich, Joseph Wu, and Radhika Jandhyala.


Bugs: MESOS-8668
https://issues.apache.org/jira/browse/MESOS-8668


Repository: mesos


Description
---

The implementation in `io::internal::read/write` uses `io::poll`, which
is UNIX specific. The Windows IOCP implementation will not use a
polling function, since there no unified mechanism, so the functions
have been moved to their own file.


Diffs
-

  3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
  3rdparty/libprocess/src/CMakeLists.txt 
cf443dffd0663ecf02b7efd6f7094175b94aae19 
  3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
  3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 
  3rdparty/libprocess/src/poll_io.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67388/diff/1/


Testing
---


Thanks,

Akash Gupta