----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46824/#review153049 -----------------------------------------------------------
src/local/main.cpp (lines 48 - 63) <https://reviews.apache.org/r/46824/#comment222316> Is there a reason why we don't just add this directly to `local::Flags`? src/master/main.cpp (lines 123 - 168) <https://reviews.apache.org/r/46824/#comment222319> Similar to above, why don't we just add these to `master::Flags`? src/master/main.cpp (lines 179 - 182) <https://reviews.apache.org/r/46824/#comment222320> I'm not sure what this is? src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1563 - 1590) <https://reviews.apache.org/r/46824/#comment222322> In most other places we seem to have opted to not specify the first part. e.g., ``` ActiveUserTestHelper::Flags::Flags() { add(&Flags::user, "user", "The expected user name."); } ``` src/slave/main.cpp (lines 97 - 151) <https://reviews.apache.org/r/46824/#comment222321> One more. Can we just add these to `slave::Flags`? - Michael Park On Oct. 17, 2016, 3:24 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46824/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2016, 3:24 p.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 > 7c35c3056326a8a135e4ad944ac741cda96cab99 > 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 > >
