mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 18383: Changed Option::get to return const reference to reduce copying.
Date Mon, 24 Feb 2014 23:42:51 GMT

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



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

    Since we maintain stout as a separate subproject, we try to avoid library dependencies
where we can. In this case, we don't want someone using Option to also have to be using google-logging.
    
    You'll notice we still are violating this in some of our old code (like os.hpp), where
there are some old-school functions like getenv that do a LOG(FATAL). :(
    
    At one point we were going to add stack trace functionality to stout so that we could
do better assertions in places like Option::get(). For now, can you keep it as an assert?



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

    Why did you need to add these? If the goal was to provide these automatically for those
including option.hpp, it might be good to spell that out here.
    
    Benh added Some and None, can you check with him as to why he didn't add these includes
implicitly?



3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp
<https://reviews.apache.org/r/18383/#comment65795>

    Ditto here.



3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp
<https://reviews.apache.org/r/18383/#comment65796>

    Perhaps we should make this optimization separately? The error case tends to be an exceptional
code path, so perhaps we can optimize this independently only if there's a need?


- Ben Mahler


On Feb. 22, 2014, 12:17 a.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18383/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2014, 12:17 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1008
>     https://issues.apache.org/jira/browse/MESOS-1008
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 3305d1363adfffc5e54ff1617e6e7c3c29f7e200

>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp d99b75aeae10319b574c67beeb6023358cac7aec

> 
> Diff: https://reviews.apache.org/r/18383/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message