Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a quiet option to the list command #75

Merged
merged 4 commits into from
Jan 5, 2016

Conversation

rgreinho
Copy link
Contributor

The use case behind it is to be able to run the tests over all the available
platforms in one line:

molecule list -q | xargs -n1 molecule test --platform

This PR also raised 2 questions:

  1. Why does the list command print an empty line first?
  2. Why does the list command have the [--platform=<platform>] [--provider=<provider>] options?

@retr0h
Copy link
Contributor

retr0h commented Dec 31, 2015

This PR also raised 2 questions:

  1. Why does the list command print an empty line first?
  2. Why does the list command have the [--platform=] [--provider=] options?

Maybe @abrown-sg has more insight on this.

@identifystation
Copy link
Contributor

The extra line is completely unnecessary. I probably threw it in to make things stand out more visually, but it's problematic when trying to parse the output of the command. I think it's fine to remove it rather than have an if statement for it. There's probably a few other commands that have this behavior that could also be cleaned up.

Not sure why list has --platform and --provider, those should be removed.

Should we consider naming the flag for formatting the output of commands to be machine readable something other than -q? To me, "quiet" generally indicates there will be no output. Not sure if there's an existing convention for that.

@rgreinho
Copy link
Contributor Author

rgreinho commented Jan 5, 2016

Ok, I'm going to fix the empty line and adjust the unused arguments.

Regarding the -q option I took the pattern from Docker, for instance in docker images or docker ps:

$ docker ps --help

Usage:  docker ps [OPTIONS]

List containers

[...]
  -q, --quiet=false     Only display numeric IDs
[...]

But you're right, for pip the -q means less verbose:

$ pip --help

Usage:
  pip <command> [options]

[...]
  -q, --quiet                 Give less output.
 [...]

I'm totally fine with replacing the -q with something else, we will just have to stick with it afterward.

Maybe that could be controlled by the verbosity argument? We could adopt this convention for all our commands:
0 v: displays nothing
1 v: displays minimal amount of information (like what I did with the -q)
2 v: full display (like what we currently have for the list command)
3 v: full display + debug info

Rémy Greinhofer added 3 commits January 5, 2016 11:28
* Add .idea directory
* Sort the file using ``sort -f .gitignore``
The list command did not use the ``platform`` and ``provider``
arguments, therefore they got removed.

There for no need of printing an empty line before displaying the list
of available platforms, therefore it got removed as well.
@dhutty
Copy link
Contributor

dhutty commented Jan 5, 2016

Traditionally:

  1. the default output of an (action) command should be nothing if it succeeded.
  2. adding 'v' options (one or more), adds verbosity. We should probably accord with the standard Pythonic sequence: critical, error, warning, info, debug unless we feel we have reason to do otherwise, in which case we should use syslog(1) levels.
  3. '--debug' or similar is an alias for the highest verbosity
  4. '-q,--quiet' is used to make the command more suitable for scripts/pipes, where exit codes should be utilised to indicate status. Usually by ensuring any output is a) minimal b) machine-readable.
  5. Sometimes -q is an alias for "less output than default" or "negate any previously set verbosity options"

However:

  • Rule 1 does not apply to non-action, 'request-for-info' commands, the purpose of which is to create output.
  • Just because Rule 1 does not apply does not mean we should ignore the other guidelines above, modifying the semantics from "log level" to "output detail". I suggest that for "request-for-info" (sub)commands, we should have something like:
    1. trivial to parse, machine-readable output, with features such as one-per-line, IDs instead of names, no fieldnames/titles/formatting, etc. (Sometimes, a '-m' option is used for this)
    2. standard, human-readable output as the default (Sometimes, a '-h' option is used for this)
    3. And if necessary, we can offer another option that can be applied to both the human or machine readable forms for the [list of] fields that should be printed, such as platform, provider, etc.

@rgreinho
Copy link
Contributor Author

rgreinho commented Jan 5, 2016

I like that explanation! 👍

Just I think 7 levels of verbosity might be a bit excessive (molecule test -vvvvvvv). I think we should limit it to 4 like ansible does.

Action items:

  • I'll replace -q with -m then for machine readable arguments.
  • -h is used for help already so, human readable should just be the default output for all the commands.
  • We should stick with those 2 rules for all the commands.

Machine readable outputs will be triggered by the use of the ``-m``
switch instead of ``-q``.
@identifystation
Copy link
Contributor

👍

rgreinho added a commit that referenced this pull request Jan 5, 2016
Add a quiet option to the list command
@rgreinho rgreinho merged commit 6c8b16f into ansible:master Jan 5, 2016
@rgreinho rgreinho deleted the quiet-list branch January 5, 2016 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants