mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 50912: Added the infrastructure for a new python-based CLI.
Date Wed, 05 Oct 2016 00:17:33 GMT

-----------------------------------------------------------
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
> 
>


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