You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/05/22 01:45:57 UTC
Re: Review Request 34278: Refactor Stout Result leveraging
Try> to remove the dynamic allocation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34278/
-----------------------------------------------------------
(Updated May 21, 2015, 11:45 p.m.)
Review request for mesos, Benjamin Hindman, Bernd Mathiske, Cody Maloney, Joerg Schad, and Michael Park.
Bugs: MESOS-2740
https://issues.apache.org/jira/browse/MESOS-2740
Repository: mesos
Description
-------
Aggregate a Try<Option<T>> to leverage the RAII pattern around unrestricted union.
Added some comments to Result<T>.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 96b0f36104bed28cc227904e34cca166435a3d38
Diff: https://reviews.apache.org/r/34278/diff/
Testing
-------
make check.
valgrind reports fewer allocations.
Thanks,
Joris Van Remoortere
Re: Review Request 34278: Refactor Stout Result leveraging
Try> to remove the dynamic allocation.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34278/#review85199
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Hindman
On May 21, 2015, 11:45 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34278/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Cody Maloney, Joerg Schad, and Michael Park.
>
>
> Bugs: MESOS-2740
> https://issues.apache.org/jira/browse/MESOS-2740
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Aggregate a Try<Option<T>> to leverage the RAII pattern around unrestricted union.
> Added some comments to Result<T>.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 96b0f36104bed28cc227904e34cca166435a3d38
>
> Diff: https://reviews.apache.org/r/34278/diff/
>
>
> Testing
> -------
>
> make check.
> valgrind reports fewer allocations.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 34278: Refactor Stout Result leveraging
Try> to remove the dynamic allocation.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34278/#review85173
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp
<https://reviews.apache.org/r/34278/#comment136689>
Please insert a blank line before the return statement.
3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp
<https://reviews.apache.org/r/34278/#comment136691>
IMHO multiple statements read better if not on the same line. Even though the original code has them on the same line. The extra parens introduced with isError() add to this impression now.
3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp
<https://reviews.apache.org/r/34278/#comment136686>
Although the second sentence is true and emphasizes an impressive point, being a _relative_ statement, it will be confusing without presenting the inferior alternative, which will be overwritten by then. Suggestion: delete it.
- Bernd Mathiske
On May 21, 2015, 4:45 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34278/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 4:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Cody Maloney, Joerg Schad, and Michael Park.
>
>
> Bugs: MESOS-2740
> https://issues.apache.org/jira/browse/MESOS-2740
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Aggregate a Try<Option<T>> to leverage the RAII pattern around unrestricted union.
> Added some comments to Result<T>.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 96b0f36104bed28cc227904e34cca166435a3d38
>
> Diff: https://reviews.apache.org/r/34278/diff/
>
>
> Testing
> -------
>
> make check.
> valgrind reports fewer allocations.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>