CLI Commands Must Not Be Half-Assed

juju
1

CLI Commands Must Not Be Half-Assed

They’re what our users see. For $importantThing’s sake, please, think
them through. Consider expectations, and conventions, and just about
every possible form of misuse you can imagine.

In particular, misunderstanding the nature of the cmd.Command interface
will trip you up and lead to you badly undertesting your commands. They
have multiple responsibilities: they are responsible both for
interpreting user intent, by parsing arguments, and by executing
that intent by communicating with a controller and one or more of its
external facades.

Both of these responsibilities are important, and conflating them makes
your life harder: the fact that it’s convenient to put the two
responsibilities into a single type does not necessarily make it
sensible to erase their distinctions when validating how they work. So,
strongly consider extracting an exported, embedded type that
implements Run on its own; and using SetFlags and Init purely to
configure that type. You can then exercise your arg-parsing in detail,
and ensure it only generates valid Run types; and exercise your Run
method in detail, secure that it can’t be affected by any internal state
that the SetFlags/Init phase might have set. (Composition FTW!)

CLI Implementation Thoughts

DO NOT USE GLOBAL VARIABLES (like os.Stdout and os.Stderr). You’re
supplied with a cmd.Context; use it, and test how it’s used. Also:

  • your Info params should be documented consistently with the rest of
    the codebase; don’t include options, they’re generated automatically.
  • stderr is for telling a human what’s going on; stdout is for
    machine-consumable results. don’t mix them up.
  • if you write to stdout, make sure you implement a --format flag and
    accept both json and yaml. (don’t write your own, see cmd.Output)
  • positional args should generally not be optional: if something’s
    optional, represent it as an option.
    • positional args can be optional, I suppose, but basically
      only when we decide that the rest of the command is in a
      position to perfectly infer intent; and that decision should not
      be taken too lightly. Before accepting it, come up with a clear
      reason why it shouldn’t be an option with a default value, and
      write that down somewhere: in the comments, at least, if not the
      documentation.

…and again, please, test this stuff to absolute destruction.
Determining user intent is quite hard enough a problem without mixing
execution concerns in.