----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5854/#review9008 -----------------------------------------------------------
src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19166> s/./)./ src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19179> 2 types to allow for implicit conversions? - naively you'd want the same type for the value and the default src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19181> The testcase does not show off the value of this piece and there is no doc doing similar. I assume this is to allow for modules deifining a flag set in a FlagsBase and then a main aggregating a few modules flags into one parse. Docs would be good as would a test case that exercises the value-add as well as the dup detection between aggregated FlagsBase's src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19182> Can't you skip the check of Flags1 since FlagsBase already does an internal dup check? src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19178> Couldn't you just specialize add for bool? I had the general impression it was a win to not link rtti when you don't have to. src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19177> kill trailing ws src/flags/flags.hpp <https://reviews.apache.org/r/5854/#comment19176> *option may point to an initialized variable in which case you leak here - it may be worth a std:cerr to warn of bad library usage. I guess you could also delete. src/tests/flags_tests.cpp <https://reviews.apache.org/r/5854/#comment19183> It would be nice to be able to test dup handling with a pluggable abort() strategy - this would address a TODO as well. - John Sirois On July 9, 2012, 10:01 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5854/ > ----------------------------------------------------------- > > (Updated July 9, 2012, 10:01 p.m.) > > > Review request for mesos, John Sirois and Vinod Kone. > > > Description > ------- > > The first of 13 reviews related to replacing configuration in Mesos with a > "flags" abstraction. In particular, the flags abstraction: > > (1) Eliminates the need to specify defaults in more than one place. > (2) Eliminates the need to specify types for options in more than one place. > > > Diffs > ----- > > src/Makefile.am 11f6b4c > src/common/strings.hpp fbca257 > src/flags/flags.hpp PRE-CREATION > src/tests/flags_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/5854/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
