> On March 26, 2016, 8:07 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, line 33
> > <https://reviews.apache.org/r/45311/diff/1/?file=1314159#file1314159line33>
> >
> >     With this default parameter the code has to be changed on every place 
> > where we return or handle a WindowsError object. 
> >     
> >     One sugestion is to use different defaults on Windows and Posix and 
> > handle the conversion from ErrnoError/Error to WindowsError inside 
> > WindowsError class.

> With this default parameter the code has to be changed on every place where 
> we return or handle a WindowsError object. 

This is not true. In places that return a `Try<T>`, we only care about the 
error string, not the error code.
Therefore, this is backwards-compatible, non-intrusive, and 
no-worse-than-before, and still allows us to be more specific going forward.
> One sugestion is to use different defaults on Windows and Posix and handle 
> the conversion from ErrnoError/Error to WindowsError inside WindowsError 
> class.

We could take this suggestion within the `os` namespace, but not at this level.
An error code such as `errno` is only present and relevant when we interact 
with the C API, which is encapsulated within the `os` namespace.

Wihtin the `os` namespace we could do something like:

```cpp
namespace os {

  using OsError =
    #ifdef __WINDOWS__
    WindowsError;
    #else
    ErrnoError;
    #endif

  template <typename T>
  using Try = ::Try<T, OsError>;

} // namespace os {
```

But again, I think this is future work, and also out of the scope of this 
review.


- Michael


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


On March 24, 2016, 8:23 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45311/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> c444c0118d39ee6a5da4618d7c62784464377280 
> 
> Diff: https://reviews.apache.org/r/45311/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to