Re: Review Request 45311: Added an additional template parameter to 'Try' for typeful error.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


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
> 
>

Re: Review Request 45311: Added an additional template parameter to 'Try' for typeful error.

[Michael Park]

> On March 26, 2016, 8:07 p.m., Daniel Pravat wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, line 33
> > 
> >
> > 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`, 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 
  using Try = ::Try;

} // 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
> 
>

Re: Review Request 45311: Added an additional template parameter to 'Try' for typeful error.

[Daniel Pravat]

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




3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp (line 33)


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.


- Daniel Pravat


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
> 
>