> On April 17, 2014, 10:09 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/include/process/metrics/metric.hpp, lines 58-60
> > <https://reviews.apache.org/r/20015/diff/8/?file=561399#file561399line58>
> >
> >     Would it be better to use an Option<Owned<...>> instead of relying on 
> > NULL?
> 
> Dominic Hamon wrote:
>     the comment explains why 'Option' doesn't work here. I can't get the 
> option and then call 'set'.
> 
> Ben Mahler wrote:
>     The comment is describing the issue with Option<TimeSeries<...>> but I'm 
> referring to Option<Owned<...>> which should work?
> 
> Dominic Hamon wrote:
>     Option<Owned<...>> doesn't work for exactly this reason. In push:
>     
>     data->history.get()->set(...)
>     
>     tries to call 'set' on a const Owned<...>&.
>     
>     Owned<Option<...>> won't work for the same reason, as it happens.
>     
>     I could decompose it to:
>     
>     Owned<...> history = data->history.get();
>     history->set(...);
>     data->history = history;
>     
>     but i'm nervous about the copying there...

I'm not sure you understood correctly:

"data->history.get()->set(...) tries to call 'set' on a const Owned<...>&"

This is actually trying to call 'operator ->() const' on a const Owned<...>& 
(which is ok), and this returns a non-const reference of TimeSeries which you 
can manipulate.


- Ben


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


On April 17, 2014, 10:27 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20015/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 10:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-1036
>     https://issues.apache.org/jira/browse/MESOS-1036
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> see summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> f4774ada4dbe3fa18b5a8b204f97f59ca015b3c1 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp 
> 4f5c1086ac3553319431283165c5451df1a0ee3f 
>   3rdparty/libprocess/include/process/metrics/metric.hpp 
> ea64f699fd9ec38745d84c7523133709827f96db 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 0cc9f4bcbbb03ac3a9a2d57f64b944443fcb94bb 
> 
> Diff: https://reviews.apache.org/r/20015/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>

Reply via email to