Re: Review Request 34644: Update existing lambdas to meet style guide

[Benjamin Hindman]

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

Ship it!


Ship It!

- Benjamin Hindman


On June 1, 2015, 5:45 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34644/
> ---
> 
> (Updated June 1, 2015, 5:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2670
> https://issues.apache.org/jira/browse/MESOS-2670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update existing lambdas to meet style guide
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 67e582cc250a9767a389e2bd0cc68985477f3ffb 
> 
> Diff: https://reviews.apache.org/r/34644/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 34644: Update existing lambdas to meet style guide

[Joris Van Remoortere]

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

Ship it!


Thanks for doing this! We'll clean up the nit when we commit.


3rdparty/libprocess/src/tests/process_tests.cpp


Comments end with a period '.'


- Joris Van Remoortere


On June 1, 2015, 5:45 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34644/
> ---
> 
> (Updated June 1, 2015, 5:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2670
> https://issues.apache.org/jira/browse/MESOS-2670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update existing lambdas to meet style guide
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 67e582cc250a9767a389e2bd0cc68985477f3ffb 
> 
> Diff: https://reviews.apache.org/r/34644/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 34644: Update existing lambdas to meet style guide

[haosdent huang]

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

(Updated June 1, 2015, 5:45 p.m.)


Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description
---

Update existing lambdas to meet style guide


Diffs (updated)
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
67e582cc250a9767a389e2bd0cc68985477f3ffb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 34644: Update existing lambdas to meet style guide

[haosdent huang]

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

(Updated June 1, 2015, 5:45 p.m.)


Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description
---

Update existing lambdas to meet style guide


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
67e582cc250a9767a389e2bd0cc68985477f3ffb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 34644: Update existing lambdas to meet style guide

[haosdent huang]

> On May 25, 2015, 10:51 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1411
> > 
> >
> > I don't think this will compile.
> > This forces a copy of `receiver`, which the `EventReceiver` class does 
> > not support.
> > Even if it did support it, we would need to mark the lambda as 
> > `mutable` since we intend to call a non `const` function on a copied value.
> > One way to resolve this is to capture `receiver` by reference. This 
> > would work since we wait till the functions are completed before we destroy 
> > the `receiver` object. Since this is somewhat dangerous and non-default 
> > behavior, we should comment what is going on if we go this way.
> > 
> > This issue arises due to the underlying transformation. The original 
> > code binds the function call the to instance of the `receiver` object. It 
> > also had the same un-documented dangerous behavior.
> > 
> > The same goes for `event2` below.
> 
> haosdent huang wrote:
> Agree here, seems a lot of lambda::bind need use capture reference if we 
> want to convert it. And it's danger. I also find some method which don't have 
> params or only have constant param, change some of them could imporve the 
> readability. There are listed in the email which I send you before.

@jvanremoortere I discard this patch. Let me continue to find other exist 
lambdas.


- haosdent


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


On May 24, 2015, 4:53 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34644/
> ---
> 
> (Updated May 24, 2015, 4:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2670
> https://issues.apache.org/jira/browse/MESOS-2670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update existing lambdas to meet style guide
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 67e582cc250a9767a389e2bd0cc68985477f3ffb 
> 
> Diff: https://reviews.apache.org/r/34644/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 34644: Update existing lambdas to meet style guide

[haosdent huang]

> On May 25, 2015, 10:51 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, line 1411
> > 
> >
> > I don't think this will compile.
> > This forces a copy of `receiver`, which the `EventReceiver` class does 
> > not support.
> > Even if it did support it, we would need to mark the lambda as 
> > `mutable` since we intend to call a non `const` function on a copied value.
> > One way to resolve this is to capture `receiver` by reference. This 
> > would work since we wait till the functions are completed before we destroy 
> > the `receiver` object. Since this is somewhat dangerous and non-default 
> > behavior, we should comment what is going on if we go this way.
> > 
> > This issue arises due to the underlying transformation. The original 
> > code binds the function call the to instance of the `receiver` object. It 
> > also had the same un-documented dangerous behavior.
> > 
> > The same goes for `event2` below.

Agree here, seems a lot of lambda::bind need use capture reference if we want 
to convert it. And it's danger. I also find some method which don't have params 
or only have constant param, change some of them could imporve the readability. 
There are listed in the email which I send you before.


- haosdent


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


On May 24, 2015, 4:53 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34644/
> ---
> 
> (Updated May 24, 2015, 4:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2670
> https://issues.apache.org/jira/browse/MESOS-2670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update existing lambdas to meet style guide
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 67e582cc250a9767a389e2bd0cc68985477f3ffb 
> 
> Diff: https://reviews.apache.org/r/34644/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 34644: Update existing lambdas to meet style guide

[Joris Van Remoortere]

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



3rdparty/libprocess/src/tests/process_tests.cpp


I don't think this will compile.
This forces a copy of `receiver`, which the `EventReceiver` class does not 
support.
Even if it did support it, we would need to mark the lambda as `mutable` 
since we intend to call a non `const` function on a copied value.
One way to resolve this is to capture `receiver` by reference. This would 
work since we wait till the functions are completed before we destroy the 
`receiver` object. Since this is somewhat dangerous and non-default behavior, 
we should comment what is going on if we go this way.

This issue arises due to the underlying transformation. The original code 
binds the function call the to instance of the `receiver` object. It also had 
the same un-documented dangerous behavior.

The same goes for `event2` below.


- Joris Van Remoortere


On May 24, 2015, 4:53 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34644/
> ---
> 
> (Updated May 24, 2015, 4:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2670
> https://issues.apache.org/jira/browse/MESOS-2670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update existing lambdas to meet style guide
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 67e582cc250a9767a389e2bd0cc68985477f3ffb 
> 
> Diff: https://reviews.apache.org/r/34644/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 34644: Update existing lambdas to meet style guide

[haosdent huang]

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

Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description
---

Update existing lambdas to meet style guide


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
67e582cc250a9767a389e2bd0cc68985477f3ffb 

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


Testing
---


Thanks,

haosdent huang