You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2014/07/24 04:55:04 UTC
Review Request 23874: Removed unused code from future.hpp.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23874/
-----------------------------------------------------------
Review request for mesos and Jiang Yan Xu.
Repository: mesos-git
Description
-------
The fail() utility is present in the mesos code as well, but usually involves deleting the promises and clearing the container.
This version inside future.hpp is unused. It would be best to remove it in favor of exposing more useful utilities in libprocess or mesos.
Diffs
-----
3rdparty/libprocess/include/process/future.hpp 5b0ed9b6fdb7e1a4b3250509ee83eb057e3696e5
Diff: https://reviews.apache.org/r/23874/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 23874: Removed unused code from future.hpp.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23874/#review48689
-----------------------------------------------------------
Ship it!
Ship It!
- Jiang Yan Xu
On July 23, 2014, 7:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23874/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 7:55 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fail() utility is present in the mesos code as well, but usually involves deleting the promises and clearing the container.
>
> This version inside future.hpp is unused. It would be best to remove it in favor of exposing more useful utilities in libprocess or mesos.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp 5b0ed9b6fdb7e1a4b3250509ee83eb057e3696e5
>
> Diff: https://reviews.apache.org/r/23874/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 23874: Removed unused code from future.hpp.
Posted by Ben Mahler <be...@gmail.com>.
> On July 24, 2014, 5:41 p.m., Jiang Yan Xu wrote:
> > You are right that it would be inconvenient if the caller still needs to loop through the collection to delete the promises. Do you think it should be in a file other than future.hpp? Maybe futures.hpp? Should be select(futures) be moved there too?
> >
> > Anyway, it feels strange to leave discard(futures) behind. I agree that we don't need to deal with this right now so maybe this can be done together later (without removing fail(futures) now)?
I left discard(futures) because it is currently used in a few locations within mesos.
There is an existing TODO for a futures namespace (possibly within another header):
https://github.com/apache/mesos/blob/0.19.1/3rdparty/libprocess/include/process/future.hpp#L845
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23874/#review48662
-----------------------------------------------------------
On July 24, 2014, 2:55 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23874/
> -----------------------------------------------------------
>
> (Updated July 24, 2014, 2:55 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fail() utility is present in the mesos code as well, but usually involves deleting the promises and clearing the container.
>
> This version inside future.hpp is unused. It would be best to remove it in favor of exposing more useful utilities in libprocess or mesos.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp 5b0ed9b6fdb7e1a4b3250509ee83eb057e3696e5
>
> Diff: https://reviews.apache.org/r/23874/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 23874: Removed unused code from future.hpp.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23874/#review48662
-----------------------------------------------------------
You are right that it would be inconvenient if the caller still needs to loop through the collection to delete the promises. Do you think it should be in a file other than future.hpp? Maybe futures.hpp? Should be select(futures) be moved there too?
Anyway, it feels strange to leave discard(futures) behind. I agree that we don't need to deal with this right now so maybe this can be done together later (without removing fail(futures) now)?
- Jiang Yan Xu
On July 23, 2014, 7:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23874/
> -----------------------------------------------------------
>
> (Updated July 23, 2014, 7:55 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The fail() utility is present in the mesos code as well, but usually involves deleting the promises and clearing the container.
>
> This version inside future.hpp is unused. It would be best to remove it in favor of exposing more useful utilities in libprocess or mesos.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/future.hpp 5b0ed9b6fdb7e1a4b3250509ee83eb057e3696e5
>
> Diff: https://reviews.apache.org/r/23874/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>