----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54486/#review158513 -----------------------------------------------------------
src/slave/containerizer/mesos/io/switchboard_main.cpp (lines 71 - 76) <https://reviews.apache.org/r/54486/#comment229281> Using sentinel values in the parameter domain as default values seems wrong to me. If you want to detect explicitly given flags you should just use an `Option<T>` for the parameter. This verification then becomes the semantically clearer if (flags.stdin_to_fd.isNone() || ... With that no default value in the `add()` calls is required. You should probably also adjust the error message to something like `Missing requireed parameters` and output `flags.usage()` after that. src/slave/containerizer/mesos/io/switchboard_main.cpp (line 79) <https://reviews.apache.org/r/54486/#comment229282> Unrelated to this patch, but this executable should probably also output usage information if invoked with `--help` (sadly this isn't handled on the `flags::FlagsBase` level). The canonical form is if (flags.help) { cerr << flags.usage(); return EXIT_SUCCESS; } - Benjamin Bannier On Dec. 7, 2016, 6:06 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54486/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2016, 6:06 p.m.) > > > Review request for mesos, Benjamin Bannier and Jie Yu. > > > Bugs: MESOS-6726 > https://issues.apache.org/jira/browse/MESOS-6726 > > > Repository: mesos > > > Description > ------- > > Added default values for all required IOSwitchboardServerFlags. > > > Diffs > ----- > > src/slave/containerizer/mesos/io/switchboard.hpp > f691b182d4029a15bbb3b1c158b176d43f265cc1 > src/slave/containerizer/mesos/io/switchboard_main.cpp > 4a9ed22ca63f0aa55ebbf7b85404a1b168a8e34a > > Diff: https://reviews.apache.org/r/54486/diff/ > > > Testing > ------- > > GTEST_FILTER="" make -j check > GTEST_FILTER="*IOSwitchboard*" src/mesos-tests > > > Thanks, > > Kevin Klues > >
