> On April 17, 2014, 3: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...
>
> Ben Mahler wrote:
> 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.
It was actually the 'reset' that was failing. Changed that to an assignment instead and all
seems good.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20015/#review40701
-----------------------------------------------------------
On April 17, 2014, 4:16 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, 4:16 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
>
>
|