From 8ea6f1c7c58d03cbbc8c51778b46ce08859d021f Mon Sep 17 00:00:00 2001 From: Anthony Keenan Date: Sun, 18 Nov 2018 12:36:21 +0000 Subject: [PATCH] [CORDA-2004]: Update CLI UX guidelines for backwards compatibility (#4248) * Update CLI UX guidelines for backwards compatibility * De-lousing * Spelling mistake --- docs/source/cli-ux-guidelines.rst | 90 +++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/docs/source/cli-ux-guidelines.rst b/docs/source/cli-ux-guidelines.rst index 41b3ac81df..d22dd8cc26 100644 --- a/docs/source/cli-ux-guidelines.rst +++ b/docs/source/cli-ux-guidelines.rst @@ -73,14 +73,18 @@ Adding a new option Parameter stability ~~~~~~~~~~~~~~~~~~~ -* Avoid removing parameters. If, for some reason, a parameter needs to be renamed, add a new parameter with the new name and deprecate the old parameter, or alternatively keep both versions of the parameter. +* Avoid removing parameters. If, for some reason, a parameter needs to be renamed, add a new parameter with the new name and deprecate the old parameter, or alternatively +keep both versions of the parameter. See :ref:`cli-ux-backwards-compatibility` for more information. +Notes for adding a new a command line application +------------------------------------------------- + The ``CordaCliWrapper`` base class ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The ``CordaCliWrapper`` base class from the ``cliutils`` module should be used as a base where practicable, this will provide a set of default options out of the box. -In order to use it, create a class containing your command line options using the syntax provided at (see the `picocli`_ website for more information) +In order to use it, create a class containing your command line options using the syntax provided at (see the `picocli `_ website for more information) .. container:: codeset @@ -133,7 +137,7 @@ Then in your ``main()`` method: Application behavior --------------------- +~~~~~~~~~~~~~~~~~~~~ * Set exit codes using exitProcess. @@ -144,4 +148,82 @@ Application behavior * Make sure all exit codes are documented with recommended remedies where applicable. * Your ``--help`` text or other docs should ideally include examples. Writing examples is also a good way to find out if your program requires a dozen flags to do anything. * Don’t print logging output to the console unless the user requested it via a ``-–verbose`` flag (conventionally shortened to ``-v``). Logs should be either suppressed or saved to a text file during normal usage, except for errors, which are always OK to print. -* Don't print stack traces to the console. Stack traces can be added to logging files, but the user should see as meaningful error description as possible. \ No newline at end of file +* Don't print stack traces to the console. Stack traces can be added to logging files, but the user should see as meaningful error description as possible. + +.. _cli-ux-backwards-compatibility: + +Backwards Compatibility +----------------------- + +Our commitment to API stability (See :doc:`api-scanner` for more information) extends to new versions of our CLI tools. Removing and renaming +parameters may cause existing scripts users may have written to fail, and should be avoided unless absolutely necessary. + +Deprecating command line parameters +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Command line parameters that are no longer necessary should be deprecated rather than removed. Picocli allows parameters to be hidden by use +of the ``hidden`` option, for example: + +.. container:: codeset + + .. sourcecode:: kotlin + + import net.corda.cliutils.CordaCliWrapper + + class UsefulUtility : CordaCliWrapper("useful-utility", "A command line utility that is super useful!") { + @Option(names = ["--no-longer-useful", "-u"], + hidden = true, + description = ["The option is no longer useful. Don't show it in the help output."] + ) + private var noLongerUseful: Boolean = false + + override fun runProgram(): Int { + if (noLongerUseful) // print a warning to the log to let the user know the option has been deprecated + logger.warn("The --no-longer-useful option is deprecated, please use the --alternatively-useful option instead") + // do some stuff + return UsefulUtilityExitCodes.SUCCESS + } + } + +This will cause the option to still be usable, but means it won't be shown when ``--help`` is called. As a result, existing scripts dependent +on the parameter will still run, but new users will be directed to the replacement. + +Changing the type of existing command line parameters +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Don't change the type of an existing command line parameter if that change would not be backwards compatible. For example, adding a +value to an enumeration based parameter would be fine, but removing one would not. Instead of changing the type, consider adding a new parameter, +deprecating the old parameter as described above, and redirecting inputs from the old parameter to the new parameter internally. + +Testing backwards compatibility +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When adding a new command line tool, a backwards compatibility test should be created by adding the ``test-cli`` as a test dependency of your project +and then creating a test class that extends ``CliBackwardsCompatibleTest`` for the class, like so: + +.. container:: codeset + + .. sourcecode:: kotlin + + import net.corda.testing.CliBackwardsCompatibleTest + + class UsefulUtilityBackwardsCompatibleTest : CliBackwardsCompatibleTest(UsefulUtility::class.java) + +The test will search for a YAML file on the class path named ``.yml`` which details the names, types and possible +options of parameters, and compares it to the options of the current class to make sure they are compatible. + +In order to generate the file, create and run the test for your application. The test will fail, but the test output +will contain the YAML for the current state of the tool. This can be copied and then pasted into a correctly named ``.yml`` +file in the resources directory of the project. + +Release process +~~~~~~~~~~~~~~~ + +As part of the release process, the release manager should regenerate the YAML files for each command line tool by following the following steps: + +* Check out the release branch +* Delete the ``.yml`` file for the tool +* Re-run the backwards compatibility test for the tool +* Copy the resulting YAML from the test output +* Check out the master branch +* Replace the text in ``.yml`` with the text generated on the release branch