mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request 34193: Refactored common functionality into FlagsBase
Date Thu, 28 May 2015 11:45:48 GMT

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
<https://reviews.apache.org/r/34193/#comment137110>

    s/ by/by/



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
<https://reviews.apache.org/r/34193/#comment137108>

    Can we also include what the output ends up looking like for this? That will keep people
from having to dig into the code to figure out where newlines are added by FlagsBase automagically.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
<https://reviews.apache.org/r/34193/#comment137107>

    Please remove the '[ERROR]' part so it doesn't proliferate via copy-paste.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp
<https://reviews.apache.org/r/34193/#comment137111>

    s/Declared/declared/



3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
<https://reviews.apache.org/r/34193/#comment137112>

    This looks like a duplicated test with the 'Usage' test above?



3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp
<https://reviews.apache.org/r/34193/#comment137113>

    s/[ERROR]//


- Benjamin Hindman


On May 26, 2015, 8:47 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34193/
> -----------------------------------------------------------
> 
> (Updated May 26, 2015, 8:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2711
>     https://issues.apache.org/jira/browse/MESOS-2711
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Jira: MESOS-2711
> 
> Every program that uses stout's `FlagsBase` ends up
> re-implementing the `printUsage()` function, and adding
> a `bool help` (and associated --help flag); this functionality
> has now been refactored in the base class and is available everywhere.
> 
> This change attempts to be backward-compatible, so it
> does not alter the behavior of the program when --help is
> invoked (by, eg, automatically printing usage and exiting)
> but leaves up to the caller to check for `flags.help` and then
> decide what action to take.
> 
> There is now a default behavior for the "leader" ("Usage: <prog name> [options]")
> but the client API allows to modify that too.
> 
> Note - anywhere I found the use of the `--help` flag the behavior was the same: 
> print usage and exit (see also https://reviews.apache.org/r/34195).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp fb383b463a99924483634eebf22bf34de318f920

>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 00281195b53d2597bdb46e3fe6cd9d46a5e9b1f1

> 
> Diff: https://reviews.apache.org/r/34193/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> **NOTE** this change by itself breaks the build, because the --help is redefined everywhere
(16 places, as of last count): CL 34195 fixes that and makes all build/tests pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message