[pve-devel] [pve-manager v2 0/5] pvesh: implement 'ls' command

[Thomas Lamprecht]

Am 07/26/2018 um 11:04 AM schrieb Dietmar Maurer:
> NOTE: needs previous patches for pve-common which are not applied so far
> 
> Also contains changes for pvenode - consider them a RFC
> 
> Dietmar Maurer (5):
>    pvesh: do not use CLIHandler $option parameter, simplify code
>    pvesh: implement 'ls' command
>    pvenode: fixup output formatting
>    pvenode: use print_api_result to print certificate information
>    pvenode: use print_api_result to print acme account information
> 
>   PVE/API2/ACMEAccount.pm |   1 +
>   PVE/CLI/pvenode.pm      |  77 +++++++++------------
>   bin/pvesh               | 177 +++++++++++++++++++++++++++++++++++++-----------
>   3 files changed, 172 insertions(+), 83 deletions(-)
> 

applies and works now...

Though comparing old a new I clearly favour the old output (but that's
not news):

NEW:

# pvesh ls / 
 
             [rev_dietmar_ls_and]
┌─────────┬──────────────┐
│ name    │ capabilities │
├─────────┼──────────────┤
│ access  │ Dr---        │
├─────────┼──────────────┤
│ cluster │ Dr---        │
├─────────┼──────────────┤
│ nodes   │ Dr---        │
├─────────┼──────────────┤
│ pools   │ Dr-c-        │
├─────────┼──────────────┤
│ storage │ Dr-c-        │
├─────────┼──────────────┤
│ version │ -r---        │
└─────────┴──────────────┘

OLD:

# pvesh ls
Dr--- access
Dr--- cluster
Dr--- nodes
Dr-c- pools
Dr-c- storage
-r--- version

(output size is 863 bytes vs 79 bytes, just as a side note)

But besides disliking of the default formatting choice, the code changes 
from this and common look OK  after a shallow review (did not tested the
ACME CLI yaml print stuff though).
It'd be nice to have manager / common in a shape that we can safely bump
them again - rolling out other bug fixes, with being sure no
regressions on CLI stuff are introduced.
How do you want to proceed with this? Do you want to apply this
yourself? Or has anyone else experienced (@Wolfgang, @Fabian, @Dominik ?)
any comments or time to review this?