> On May 11, 2016, 11:27 p.m., Vinod Kone wrote: > > src/master/main.cpp, line 131 > > <https://reviews.apache.org/r/46824/diff/3/?file=1379589#file1379589line131> > > > > If `add` should be only called by derived classes, should `add` method > > be protected instead of public? > > Benjamin Bannier wrote: > I am not sure `add` should only be called from derived classes, and since > we force callers to pass in a member variable pointer these usages appear > fine to me. Since most (all?) calls to `add` are from classes derived from > `FlagsBase` making it `protected` instead of `public` would be possible, but > I wouldn't be sure why that would be needed. It might also potentially make > some valid use cases impossible.
Dropping this thightening up the interfacce of `FlagsBase` is out of scope for this changeset. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46824/#review132750 ----------------------------------------------------------- On Sept. 29, 2016, 2:55 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46824/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2016, 2:55 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 fcf627b0dcbedd01700cc8c9acadf7ba0dae4faa > src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e > src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f > src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 > src/examples/disk_full_framework.cpp > 1221f83d495f7c1ee1bcbcfe067e303db0a921eb > src/examples/dynamic_reservation_framework.cpp > c9a68637ea2dfbb0a9367f14358b5e5de46f3331 > src/examples/load_generator_framework.cpp > 5402e31b89b7ead1dc8fdc9065980b5b1c0d380c > src/examples/long_lived_framework.cpp > 7c57eb5e4a34208504475013690ae8e3cae74155 > src/examples/no_executor_framework.cpp > 57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 > src/examples/persistent_volume_framework.cpp > 3cc7cf0c4c97d90f2e70800d7a8d4ca64c2150d1 > src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d > src/examples/test_http_framework.cpp > d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 > src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 > src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 > src/master/main.cpp 4a1a8e70ab0535aa131681b2b09a99e51c20158e > src/slave/container_loggers/lib_logrotate.hpp > a8653d716a898f03cce83f7b88b666dc46c0ea90 > src/slave/container_loggers/logrotate.hpp > f906a167f8897385af5f54e1e77ddddcb790121a > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 1ee56cfa77a6b82f43882ab40d991c1937b5bd63 > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp > 01355a15a4586c4afd052251548872a6f071b42d > src/slave/containerizer/mesos/launch.cpp > 7cc0c3f46fe1a5883b7ccd474595b8be412b355c > src/slave/containerizer/mesos/mount.cpp > dbd7853a43ee1402f2f91d933a657010efdd76b0 > src/slave/main.cpp 949a738ab3e44d8efc878e59c482a750ab41d1e4 > 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 > >
