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


Hey Haosdent,
These look great.
Just 1 suggestion for cleaning up the style while we're touching these lines of 
code :-)


src/log/network.hpp
<https://reviews.apache.org/r/34018/#comment135174>

    It looks like we missed a nice example for this in the style guide! (the 
lambda not being the first function parameter argument).
    
    What do you think of re-aligning the code like this:
    
    ```
      process::collect(futures)
        .after(
            Seconds(5), 
            [](process::Future<std::list<Option<std::string>>> datas) {
              // Handling time outs when collecting membership data. For 
              // now, a timeout is treated as a failure.
              datas.discard();
              return process::Failure("Timed out");
            })
        .onAny(executor.defer(lambda::bind(&This::collected, this, 
lambda::_1)));
    ```
    
    Notice:
    1) I put the arguments to `after()` each on their own line, indented 4 
spaces deeper.
    2) I re-wrapped the comment to the 70 character boundary.
    3) I got rid of the white-space between the closing brackets: `> > >` -> 
`>>>`. Yay newer compilers :-)


- Joris Van Remoortere


On May 9, 2015, 7:52 p.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34018/
> -----------------------------------------------------------
> 
> (Updated May 9, 2015, 7:52 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
> -------
> 
> According Joris advice, replace the 'lambda::bind' expressions match these 
> rules:
> 
> 1. Binds to a static function without any side-effects.
> 2. Is self contained (i.e. does not rely on contextual parameters)
> 3. Does not bind in any arguments. (i.e. only uses lambda::_N for arguments)
> 4. Is only called in 1 place OR is so small that it is ok to repeat the code.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.cpp df3211a0c25d7a16f36814886d14f81caaef2b9c 
>   src/log/network.hpp 7c74a55cc2a71fa2acd207605f972e7fbd203be4 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> 6a5b2b5c6e2844fe1a10815956569194b6f56681 
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 2c218b2b83cf42f54dbc7ec4c2ba8960b6e194de 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> a4abaff30bb4646b1b1edfdbbc243c9e3f6851df 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f2587280dc0e1d566d2b856a80358c7b3896c603 
> 
> Diff: https://reviews.apache.org/r/34018/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to