Re: Review Request 34644: Update existing lambdas to meet style guide
--- 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 https://reviews.apache.org/r/34644/#comment139465 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
--- 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
On May 25, 2015, 10:51 p.m., Joris Van Remoortere wrote: 3rdparty/libprocess/src/tests/process_tests.cpp, line 1411 https://reviews.apache.org/r/34644/diff/1/?file=971207#file971207line1411 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
On May 25, 2015, 10:51 p.m., Joris Van Remoortere wrote: 3rdparty/libprocess/src/tests/process_tests.cpp, line 1411 https://reviews.apache.org/r/34644/diff/1/?file=971207#file971207line1411 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
Review Request 34644: Update existing lambdas to meet style guide
--- 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
