> On June 25, 2014, 4:33 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/subcommand.hpp, line 86
> > <https://reviews.apache.org/r/22764/diff/5/?file=616543#file616543line86>
> >
> > Just a thought: I'm not convinced that you might not sometimes want to
> > see all of "your" argc and argv (where I define "your" as being everything
> > after argv[0] and argv[1]). We've seen this in our code base too, see the
> > mesos-executor.
> >
> > Really what we're saving Subcommand writers is the three lines to do
> > flags.load() and check for an error by not passing them their argc and argv.
Just checked the 'override' code in mesos-executor. Yeah, I agree that in some
cases we need to access argc and argv in 'execute()'. However, I still wanna
keeps the common flags parsing part as it is right now since in most of the
cases, we don't need to access argc and argv.
I've added a TODO here (feel lazy to change all the dependent patches). We can
have something like this in the future:
virtual int execute() = 0;
// Default behavior is to call execute(). User can override this to provide
customized flags parsing.
virtual int execute(int argc, char** argv)
{
// Parsing flags using argc and argv
Try<Nothing> load = getFlags()->load(...);
...
return execute();
}
> On June 25, 2014, 4:33 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp, line 543
> > <https://reviews.apache.org/r/22764/diff/5/?file=616544#file616544line543>
> >
> > We should have an independent Subprocess Flags test and a
> > SubcommandTest so we don't conflate the two. Seems like you could
> > define/declare TestSubcommand::Flags outside of TestSubcommand (as it
> > originally was done) but still use it in TestSubcommand. Moreover, you
> > could define a helper that sets up these Flags that gets used both by the
> > original test and the SubcommandTest so that you don't have to duplicate
> > the code.
Added SubcommandTest in stout separately.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22764/#review46639
-----------------------------------------------------------
On June 25, 2014, 6:37 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22764/
> -----------------------------------------------------------
>
> (Updated June 25, 2014, 6:37 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Bugs: MESOS-1511
> https://issues.apache.org/jira/browse/MESOS-1511
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am eac7ab5
> 3rdparty/libprocess/3rdparty/stout/include/stout/subcommand.hpp
> PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 7dfa384
> src/Makefile.am 861aad2
> src/launcher/launcher.hpp 35cdc69
> src/launcher/launcher.cpp 872e2e8
> src/launcher/main.cpp b497e98
> src/tests/environment.cpp 21b9d1d
> src/tests/launcher_tests.cpp e293cc5
>
> Diff: https://reviews.apache.org/r/22764/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
