You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/07/28 02:56:18 UTC

Review Request 36868: Added -> operators for Option, Try, Result.

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

Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere.


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


Repository: mesos


Description
-------

See MESOS-2757.


Diffs
-----

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 
  3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 
  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 
  3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 
  3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 
  3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION 

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


Testing
-------

Added tests.


Thanks,

Ben Mahler


Re: Review Request 36868: Added -> operators for Option, Try, Result.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36868/#review96429
-----------------------------------------------------------

Ship it!


Adjust the operator spacing.
Keeping the current style for now, we can align it differently later if we want. I'd rather get the functionality in.
Using the recommended way to delegate from the non-const version to the const version.

- Joris Van Remoortere


On July 28, 2015, 12:56 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36868/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2757
>     https://issues.apache.org/jira/browse/MESOS-2757
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See MESOS-2757.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 
>   3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 
>   3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 
>   3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36868/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36868: Added -> operators for Option, Try, Result.

Posted by Michael Park <mc...@gmail.com>.

> On July 29, 2015, 4:42 a.m., Michael Park wrote:
> > Why wasn't [r36869](https://reviews.apache.org/r/36869) just included in this patch?

Ah, it's because this patch is `stout` whereas [r36869](https://reviews.apache.org/r/36869/) is `libprocess`.


- Michael


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


On July 28, 2015, 12:56 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36868/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2757
>     https://issues.apache.org/jira/browse/MESOS-2757
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See MESOS-2757.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 
>   3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 
>   3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 
>   3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36868/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36868: Added -> operators for Option, Try, Result.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36868/#review93388
-----------------------------------------------------------


Why wasn't [r36869](https://reviews.apache.org/r/36869) just included in this patch?


3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp (lines 109 - 110)
<https://reviews.apache.org/r/36868/#comment147756>

    Are we settled and agreed with this style of lining things up? I'm against this style because it becomes even more difficult to use automated formatting tools like `clang-format`. Here and below.



3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
<https://reviews.apache.org/r/36868/#comment147757>

    Why the move of these functions?



3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp (lines 117 - 126)
<https://reviews.apache.org/r/36868/#comment147758>

    The standard technique here to remove the duplicate code is to leverage the `const` version.
    
    ```
    T& get() {
      return const_cast<T &>(static_cast<const Result &>(*this).get());
    }
    ```


- Michael Park


On July 28, 2015, 12:56 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36868/
> -----------------------------------------------------------
> 
> (Updated July 28, 2015, 12:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2757
>     https://issues.apache.org/jira/browse/MESOS-2757
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See MESOS-2757.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 549fc46cedb643ef1ebdf8441c332a02ac45016d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 3d20614333864bff9c3801c71f2a384c4aa41a3f 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 5ad611497a47be64c539e832b9a1c23e6cf9586d 
>   3rdparty/libprocess/3rdparty/stout/tests/option_tests.cpp 0c3f89bafe1afb15d1a2d775ed598cdf1a5ea147 
>   3rdparty/libprocess/3rdparty/stout/tests/result_tests.cpp 0a381060ef418ab09b1d4ec2101d75a2a2c29e65 
>   3rdparty/libprocess/3rdparty/stout/tests/try_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36868/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>