----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50912/#review151421 -----------------------------------------------------------
Fix it, then Ship it! Builds and runs as expected. configure.ac (lines 200 - 204) <https://reviews.apache.org/r/50912/#comment219744> The inclusion of the autoconf/automake bits in this review gives the impression that we need to `make` before doing anything with the CLI. Technically, we do not even need to `../configure --enable-new-cli` in order to use the CLI. I'd prefer if the autoconf/automake things were a separate patch. src/cli_new/README.md (lines 15 - 17) <https://reviews.apache.org/r/50912/#comment219740> Given that there is already a top-level `bootstrap`, you may want to refer to `cli_new/bootstrap` in some other way. src/cli_new/bin/main.py (line 18) <https://reviews.apache.org/r/50912/#comment219756> s/mesos-cli/Mesos CLI/ src/cli_new/bin/main.py (line 32) <https://reviews.apache.org/r/50912/#comment219757> s/mesos/Mesos/ src/cli_new/bin/main.py (line 81) <https://reviews.apache.org/r/50912/#comment219753> Period at the end. src/cli_new/bootstrap (line 41) <https://reviews.apache.org/r/50912/#comment219754> Period at the end. src/cli_new/lib/mesos/exceptions.py (line 23) <https://reviews.apache.org/r/50912/#comment219766> Period at the end. src/cli_new/lib/mesos/plugins/base.py (line 71) <https://reviews.apache.org/r/50912/#comment219763> Period at the end. src/cli_new/lib/mesos/plugins/base.py (line 134) <https://reviews.apache.org/r/50912/#comment219762> Period at the end. src/cli_new/lib/mesos/util.py (lines 73 - 77) <https://reviews.apache.org/r/50912/#comment219751> It would really help to explain what the three return types mean. i.e. ``` Returns `comp_words` if the completion word is potentially in that list. Returns an empty list if there is no possible completion. Returns `None` if this autocomplete is already done. ``` src/cli_new/lib/mesos/util.py (line 107) <https://reviews.apache.org/r/50912/#comment219765> Period at the end. src/cli_new/lib/mesos/util.py (line 113) <https://reviews.apache.org/r/50912/#comment219749> s/mesos/the top-level entry point/ src/cli_new/lib/mesos/util.py (line 128) <https://reviews.apache.org/r/50912/#comment219764> Period at the end. src/cli_new/mesos.bash_completion (lines 16 - 20) <https://reviews.apache.org/r/50912/#comment219752> There are mixed tabs and spaces in this file. - Joseph Wu On Oct. 1, 2016, 1:09 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50912/ > ----------------------------------------------------------- > > (Updated Oct. 1, 2016, 1:09 p.m.) > > > Review request for mesos, Haris Choudhary and Joseph Wu. > > > Bugs: MESOS-6008 > https://issues.apache.org/jira/browse/MESOS-6008 > > > Repository: mesos > > > Description > ------- > > Added the infrastructure for a new python-based CLI. > > > Diffs > ----- > > configure.ac 57482d39db1f83e92e75fca959cd6df329a1c24f > docs/getting-started.md de3fba4a821ca7fa9b9e2fd692b33b40edd35b68 > src/Makefile.am f093000e0282a8d5ac17e7ba33711690ccdfe68a > src/cli_new/.gitignore PRE-CREATION > src/cli_new/README.md PRE-CREATION > src/cli_new/activate PRE-CREATION > src/cli_new/bin/config.py PRE-CREATION > src/cli_new/bin/main.py PRE-CREATION > src/cli_new/bin/mesos PRE-CREATION > src/cli_new/bootstrap PRE-CREATION > src/cli_new/deactivate PRE-CREATION > src/cli_new/lib/mesos/__init__.py PRE-CREATION > src/cli_new/lib/mesos/docopt.py PRE-CREATION > src/cli_new/lib/mesos/exceptions.py PRE-CREATION > src/cli_new/lib/mesos/plugins/__init__.py PRE-CREATION > src/cli_new/lib/mesos/plugins/base.py PRE-CREATION > src/cli_new/lib/mesos/util.py PRE-CREATION > src/cli_new/mesos.bash_completion PRE-CREATION > src/cli_new/pip-requirements.txt PRE-CREATION > src/cli_new/pylint.config PRE-CREATION > > Diff: https://reviews.apache.org/r/50912/diff/ > > > Testing > ------- > > ../configure --enable-new-cli > make -C src mesos > src/mesos > > > Thanks, > > Kevin Klues > >
