----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46863/#review132999 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/stout/include/stout/flags/flags.hpp (lines 661 - 663) <https://reviews.apache.org/r/46863/#comment197301> // TODO(vinod): Reject flags with an unknown name if `unknowns` is false. // This will break backwards compatibility however! 3rdparty/stout/include/stout/flags/flags.hpp (lines 664 - 665) <https://reviews.apache.org/r/46863/#comment197300> How about strings::remove with PREFIX? 3rdparty/stout/include/stout/flags/flags.hpp (line 690) <https://reviews.apache.org/r/46863/#comment197302> Since this ends up holding both command and environment values, how about just 'values' now? 3rdparty/stout/include/stout/flags/flags.hpp (lines 728 - 729) <https://reviews.apache.org/r/46863/#comment197303> How about: ``` // Merge in flags from the environment. Command-line // flags take precedence over environment flags. ``` 3rdparty/stout/include/stout/flags/flags.hpp (line 750) <https://reviews.apache.org/r/46863/#comment197305> Don't need this? 3rdparty/stout/include/stout/flags/flags.hpp (line 751) <https://reviews.apache.org/r/46863/#comment197306> s/cmdValues/values/ 3rdparty/stout/include/stout/flags/flags.hpp (lines 797 - 798) <https://reviews.apache.org/r/46863/#comment197304> // Merge in flags from the environment. Command-line // flags take precedence over environment flags. 3rdparty/stout/include/stout/flags/flags.hpp (line 831) <https://reviews.apache.org/r/46863/#comment197309> can you suffix with _ on the second instance? 3rdparty/stout/include/stout/flags/flags.hpp (lines 835 - 836) <https://reviews.apache.org/r/46863/#comment197307> Could we wrap at the paren? 3rdparty/stout/include/stout/flags/flags.hpp (line 844) <https://reviews.apache.org/r/46863/#comment197310> How about _ as a suffix on the second instance? 3rdparty/stout/include/stout/flags/flags.hpp (line 862) <https://reviews.apache.org/r/46863/#comment197308> Could we wrap at the paren? 3rdparty/stout/include/stout/flags/flags.hpp (lines 883 - 884) <https://reviews.apache.org/r/46863/#comment197311> Shouldn't this print the `name` being looped over here so that we can see the two clashing names? - Ben Mahler On May 12, 2016, 10:42 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46863/ > ----------------------------------------------------------- > > (Updated May 12, 2016, 10:42 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-5370 > https://issues.apache.org/jira/browse/MESOS-5370 > > > Repository: mesos > > > Description > ------- > > The logic is now moved to Flag::load() keep the duplicate checking logic > for names and aliases in one place. > > > Diffs > ----- > > 3rdparty/stout/include/stout/flags/flags.hpp > 02584e28b938bba738c2e32b3cb7a04a47693853 > > Diff: https://reviews.apache.org/r/46863/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
