----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26688/#review56541 -----------------------------------------------------------
This looks good to me, just a couple of questions about the tests. Speaking of, can you update testing done and add an example of the new help output? Also, David's on vacation, so you'll want to add someone else to this review. src/test/python/apache/aurora/client/cli/test_help.py <https://reviews.apache.org/r/26688/#comment96897> Are option names guaranteed to be unique? If not this test could potentially pass if any help output contains a plugin option name, not necessarily the help output for the command to which the plugin was registered. It's also possible for a plugin option name to appear in the help for another option, and not on its own, which would cause this test to succeed even if the plugin options themselves are not properly displayed? I guess what I'm getting at is would it be better to test for more than just the appearance of a string at any point in the output? (This may be based on incomplete understanding of how the client registers commands/options). src/test/python/apache/aurora/client/cli/test_help.py <https://reviews.apache.org/r/26688/#comment96898> Are there any other varieties of metavars? could we have int metavars that are similarly unset, resulting in help output that just says "int" rather than "str"? - Joshua Cohen On Oct. 14, 2014, 3:07 p.m., Mark Chu-Carroll wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26688/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2014, 3:07 p.m.) > > > Review request for Aurora, David McLaughlin and Joshua Cohen. > > > Bugs: aurora-831 > https://issues.apache.org/jira/browse/aurora-831 > > > Repository: aurora > > > Description > ------- > > - Put plugin-generated options into the correct order. > - Include the option-name in the detailed help list. > - Add missing metavars. > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/__init__.py > da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 > src/main/python/apache/aurora/client/cli/options.py > dc76c25b90acb9610e40b939e65c3cabf032649f > src/main/python/apache/aurora/client/cli/standalone_client.py > 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 > src/test/python/apache/aurora/client/cli/test_help.py > f73c8a3778b7d118ea2865f213b442a607fb4a7d > > Diff: https://reviews.apache.org/r/26688/diff/ > > > Testing > ------- > > > Thanks, > > Mark Chu-Carroll > >
