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


Fix it, then Ship it!




I'll just make the adjustments below when I commit it. Please let me know if 
you're against any of them.


src/examples/test_http_framework.cpp (lines 385 - 386)
<https://reviews.apache.org/r/46824/#comment222329>

    `&::Flags::role` seems unnecessary. Let's just do `&Flags::role`



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1583 - 1586)
<https://reviews.apache.org/r/46824/#comment222327>

    Seems like you missed this one.
    I'll commit this as:
    ```
    add(&Flags::hostname, "hostname", "Hostname of the container");
    ```



src/slave/main.cpp (lines 102 - 135)
<https://reviews.apache.org/r/46824/#comment222330>

    Similar to above. `&::Flags::ip` seems unnecessary. Let's just do 
`&Flags::ip`


- Michael Park


On Oct. 18, 2016, 8:39 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46824/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3335
>     https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -----
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
>   src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
>   src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
>   src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
>   src/examples/disk_full_framework.cpp 
> 78f13d34f183628372f925f450f76f250a5d81d1 
>   src/examples/dynamic_reservation_framework.cpp 
> c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
>   src/examples/load_generator_framework.cpp 
> 81c8c237dedd27615fe34ad629978a7359ee1efd 
>   src/examples/long_lived_framework.cpp 
> bfb253efbe80f52147123abdd804877c175e6f5e 
>   src/examples/no_executor_framework.cpp 
> 466854263cab96e764a3aedca5e59e54bdc6f500 
>   src/examples/persistent_volume_framework.cpp 
> a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
>   src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
>   src/examples/test_http_framework.cpp 
> d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
>   src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
>   src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
>   src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f91db3edc3d0cc93fedda906d175cbcca0c00c12 
>   src/slave/container_loggers/logrotate.hpp 
> f906a167f8897385af5f54e1e77ddddcb790121a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> c87e6715a3f00a6ed2f6da03bd7492f9e77ffc21 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 20fb6ab35ffa85917c6705d79f22f0d54a416353 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/containerizer/mesos/mount.cpp 
> 4b90666d9c75dadd70e6b690ca17fc5699825442 
>   src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
>   src/tests/active_user_test_helper.cpp 
> 80bd0acff47689be95a42bad3cb867e0faa68554 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 2d7be6ddaf4a5e687e928e4651df68ea9b234202 
> 
> Diff: https://reviews.apache.org/r/46824/diff/
> 
> 
> Testing
> -------
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Reply via email to