-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5863/#review9073
-----------------------------------------------------------



src/flags/flag.hpp
<https://reviews.apache.org/r/5863/#comment19211>

    In isolation this is odd coupling:
    Flag depends on FlagBase, it has no manifest value type here, but a value 
type is implied by the loader (it clearly knows the type it loads).
    It would seem more natural to have
    <typename T>
    struct Flag<T>
    {
      ...
      std::tr1::function<T(const std::string&)> parser;
    }
    



src/flags/flags.hpp
<https://reviews.apache.org/r/5863/#comment19219>

    urg - I suppose this blocks my Flag<T> suggestion :/



src/flags/flags.hpp
<https://reviews.apache.org/r/5863/#comment19218>

    It would be nice if there was an overload where I could pass a loader 
reference for those times when I have a 1-off flag type to parse



src/flags/flags.hpp
<https://reviews.apache.org/r/5863/#comment19216>

    kill



src/master/webui.hpp
<https://reviews.apache.org/r/5863/#comment19217>

    kill trailing ws


- John Sirois


On July 9, 2012, 10:22 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5863/
> -----------------------------------------------------------
> 
> (Updated July 9, 2012, 10:22 p.m.)
> 
> 
> Review request for mesos, John Sirois and Vinod Kone.
> 
> 
> Description
> -------
> 
> After converting some things over to use the new flags abstraction, I 
> realized that I didn't provide everything I really wanted, so I refactored it 
> a bit. This is really the "flags" abstraction review.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 11f6b4c 
>   src/configurator/configurator.hpp 8569645 
>   src/flags/flag.hpp PRE-CREATION 
>   src/flags/flags.hpp PRE-CREATION 
>   src/flags/loader.hpp PRE-CREATION 
>   src/logging/flags.hpp PRE-CREATION 
>   src/logging/logging.hpp PRE-CREATION 
>   src/logging/logging.cpp PRE-CREATION 
>   src/master/flags.hpp PRE-CREATION 
>   src/master/http.cpp 82784fb 
>   src/master/main.cpp bcbe35a 
>   src/master/master.hpp 886f79c 
>   src/master/master.cpp 44a5dc6 
>   src/master/webui.hpp 660d5e1 
>   src/master/webui.cpp 56134b7 
>   src/tests/flags_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/5863/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Reply via email to