Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Kevin Klues

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

(Updated Jan. 29, 2016, 10:10 a.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


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


Repository: mesos


Description
---

Previously, templates only existed to allow collect to take either a
std::list of Futures, or *exactly* 2 Futures as arguments.

This commit removes the 2 argument template and replaces it with a
variadic template that accepts an arbitrary number of Futures as
arguments.


Diffs
-

  3rdparty/libprocess/include/process/collect.hpp 
cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 

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


Testing
---


Thanks,

Kevin Klues



Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Kevin Klues

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

Review request for mesos, Joris Van Remoortere and Michael Park.


Repository: mesos


Description
---

Previously, templates only existed to allow collect to take either a
std::list of Futures, or *exactly* 2 Futures as arguments.

This commit removes the 2 argument template and replaces it with a
variadic template that accepts an arbitrary number of Futures as
arguments.


Diffs
-

  3rdparty/libprocess/include/process/collect.hpp 
cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 

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


Testing
---


Thanks,

Kevin Klues



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Kevin Klues

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

(Updated Jan. 29, 2016, 10:34 p.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.


Changes
---

Addressed comment for 's/func/f/', still waiting for feedback on the gcc bug.


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


Repository: mesos


Description
---

Previously, templates only existed to allow collect to take either a
std::list of Futures, or *exactly* 2 Futures as arguments.

This commit removes the 2 argument template and replaces it with a
variadic template that accepts an arbitrary number of Futures as
arguments.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 

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


Testing
---


Thanks,

Kevin Klues



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Ben Mahler

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


Ship it!




Thanks for fixing this debt!


3rdparty/libprocess/include/process/collect.hpp (lines 224 - 225)


Newline here?

Also, we tend to put the .then on a new line as if it was a statement, to 
make it clear that there's a continuation happening here:

```
return collect(wrappers)
  .then(std::bind(f, futures...));
```


- Ben Mahler


On Jan. 30, 2016, 1:43 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42955/
> ---
> 
> (Updated Jan. 30, 2016, 1:43 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-4551
> https://issues.apache.org/jira/browse/MESOS-4551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, templates only existed to allow collect to take either a
> std::list of Futures, or *exactly* 2 Futures as arguments.
> 
> This commit removes the 2 argument template and replaces it with a
> variadic template that accepts an arbitrary number of Futures as
> arguments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/collect.hpp 
> cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 
> 
> Diff: https://reviews.apache.org/r/42955/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Benjamin Bannier

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



LGTM, but it would be nice to include a pointer to the GCC bug 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47226) and probably also the 
workaround more prominently.

- Benjamin Bannier


On Jan. 29, 2016, 11:34 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42955/
> ---
> 
> (Updated Jan. 29, 2016, 11:34 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-4551
> https://issues.apache.org/jira/browse/MESOS-4551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, templates only existed to allow collect to take either a
> std::list of Futures, or *exactly* 2 Futures as arguments.
> 
> This commit removes the 2 argument template and replaces it with a
> variadic template that accepts an arbitrary number of Futures as
> arguments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/collect.hpp 
> cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 
> 
> Diff: https://reviews.apache.org/r/42955/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Kevin Klues

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

(Updated Jan. 30, 2016, 1:43 a.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.


Changes
---

Checks were hanging due to the wcall to futures.get()... in the bind call for 
collect().  We rearranged things to avoid this.


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


Repository: mesos


Description
---

Previously, templates only existed to allow collect to take either a
std::list of Futures, or *exactly* 2 Futures as arguments.

This commit removes the 2 argument template and replaces it with a
variadic template that accepts an arbitrary number of Futures as
arguments.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 

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


Testing
---


Thanks,

Kevin Klues



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Ben Mahler

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



Looks good, would be great if you could get some feedback from michael park on 
the gcc bug you encountered.


3rdparty/libprocess/include/process/collect.hpp (lines 43 - 44)


s/TYPES/Ts/ here and below



3rdparty/libprocess/include/process/collect.hpp (lines 217 - 225)


This looks ok to me, but could you ping mpark to see if he has any feedback 
on this?



3rdparty/libprocess/include/process/collect.hpp (line 223)


s/func/f/


- Ben Mahler


On Jan. 29, 2016, 9:18 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42955/
> ---
> 
> (Updated Jan. 29, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-4551
> https://issues.apache.org/jira/browse/MESOS-4551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, templates only existed to allow collect to take either a
> std::list of Futures, or *exactly* 2 Futures as arguments.
> 
> This commit removes the 2 argument template and replaces it with a
> variadic template that accepts an arbitrary number of Futures as
> arguments.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/collect.hpp 
> cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 
> 
> Diff: https://reviews.apache.org/r/42955/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Re: Review Request 42955: Added variadic template for process::collect.

2016-01-29 Thread Kevin Klues

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

(Updated Jan. 29, 2016, 9:18 p.m.)


Review request for mesos, Jie Yu, Joris Van Remoortere, and Michael Park.


Changes
---

GCC 4.8 has a bug using variadic templates with lambdas. This patch changes the 
previous one to use a std::bind as a workaround for this bug.

http://binglongx.com/2014/04/25/workaround-for-gcc-bug-in-lambda-capturing-variadic-templates/


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


Repository: mesos


Description
---

Previously, templates only existed to allow collect to take either a
std::list of Futures, or *exactly* 2 Futures as arguments.

This commit removes the 2 argument template and replaces it with a
variadic template that accepts an arbitrary number of Futures as
arguments.


Diffs (updated)
-

  3rdparty/libprocess/include/process/collect.hpp 
cd78b6c211c2e4ca2b1ebbe728cfc2dfad1a32c9 

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


Testing
---


Thanks,

Kevin Klues