Updated Readthedocs documentation in line with current practices.

doc/source/Code-Style-Guide.rst

@@ -37,6 +37,9 @@ General comments
   line interfaces) have acceptance tests.
 * Use type annotations on function and method interfaces, except for
   CLIs.
+* Avoid putting any functionality within the CLI at all other than
+  passing in arguments to the plugin, all functionality should be
+  done within the plugin layer itself.
 
 Pull requests
 ~~~~~~~~~~~~~
@@ -519,7 +522,7 @@ Add a command line interface (improver/cli/<cli_name>.py) to invoke plugins
 that can be used as a standalone utility or executable within a suite context
 (e.g. wind downscaling, neighbourhood processing, spot data extraction).
 These CLIs are invoked using ``bin/improver <cli-name>`` (note that the
-CLI filename uses underscores, but the call to use the CLI uses hyphens)
+CLI filename uses underscores, but the call to use the CLI uses hyphens).
 
 IMPROVER CLIs should only have ``from improver import cli`` as the top
 level imports. Other imports are placed inside the function that uses
@@ -534,6 +537,11 @@ to save a cube to disk, it will need the decorator ``@cli.with_output``,
 this will mean on the command line, the ``--output`` flag can be used to
 specify an output path.
 
+As mentioned above, it is important to ensure that no other functionality
+than passing inputs into the plugin is done within the CLI layer.
+So any checks on the data or requirements of inputs should be done in
+the plugin itself.
+
 To load the cubes, each cube argument will need a type. For a basic cube
 this will be ``cube: cli.inputcube``. If there is a default argument to
 go with the typed variable, spaces are required around the ``=`` for
@@ -765,5 +773,6 @@ New release steps:
    case just check it. The checksum of the compressed ``.tar.gz`` IMPROVER
    source code can be obtained via ``openssl sha256 <file name>``.
    Currently the people with write access to the improver-feedstock
-   repository are @benfitzpatrick, @PaulAbernethy, @tjtg and @lucyleeow.
+   repository are @benfitzpatrick, @PaulAbernethy, @tjtg, @cpelley and
+   @dementipl.
    You can ping one of these people to merge your pull request.

doc/source/Guidance-for-Reviewing-Code.rst

@@ -8,9 +8,8 @@ As a Developer:
     branch up to origin.
 2.  Check the code with respect to the standards in the Definition of
     Done. Also check that new unit tests do not raise any warnings.
-3.  Create a pull request. A main reviewer must be assigned, but adding
-    multiple reviewers to the pull request is encouraged to help share
-    expertise. Please ask for help assigning reviewers if you need it.
+3.  Create a pull request and notify a member of the team so it can be
+    assigned to an appropriate reviewer.
 4.  The relevant issue should be referenced within the pull request.
 5.  Respond to the reviewers’ comments and update the pull request
     accordingly. This will be an iterative process.
@@ -26,32 +25,28 @@ As a Developer:
     perspective, ensure that a subject matter expert is satisfied with
     the changes.
 10. (Internal staff only:) Prior to moving the issue into the 'Done'
-    column, notify the Product Owner that the issue has been completed,
-    and discuss the functionality implemented.
+    column, assign the ticket back to the original developer and notify
+    them in preparation for merging into the codebase.
 
 As a Reviewer:
 --------------
 
-1. Issues should be assigned to a main reviewer. Adding multiple
-   reviewers to the pull request is encouraged to help share expertise.
-   Please ask for help assigning reviewers if you need it.
+1. All reviewers are encouraged to add comments to the pull request.
 
-2. All reviewers are encouraged to add comments to the pull request.
-
-3. Main reviewers should:
+2. Reviewers should:
 
    a.   **Read the code and post in-line comments for suggested
         alterations.**
    b.   **Ensure unit tests are run and pass.**
    c.   **Ensure command line interface acceptance tests run and pass.
-        These must be ran on the desktop using bin/improver tests as they
-        are not run by travis**
+        These must be run on the desktop using bin/improver tests,
+        see :doc:`Running-at-your-site` for more information**
    d.   **The Acceptance Criteria defined within the associated issue has
         been met.**
    e.   **The criteria within the Definition of Done has been satisfied.**
    f.   **Ensure their testing is documented on the issue.**
 
-4. Main reviewers should post comments to the pull request to show that
+4. Reviewers should post comments to the pull request to show that
    they have completed: a, b, c, d, e, f.
 
 5. Things to consider when reading through the code are:
@@ -80,6 +75,7 @@ As a Reviewer:
 
         * Does the code do what is supposed to?
         * Are errors handled appropriately?
+        * Is the code written to be run efficiently?
 
    e.   **Test coverage**
 
@@ -92,19 +88,6 @@ As a Reviewer:
    criteria listed above have been met. The Scrum Master can be
    consulted, if necessary.
 
-7. If this is a second review, the developer should contact the Product
-   Owner prior to moving the issue into the 'Done' column. The Scrum
-   Master can be consulted, if necessary.
-
-Before merging:
----------------
-
-Always rerun Travis before merging a PR. Do this by:
-
-1. At the bottom of the PR, click ``show all checks`` and then ``details`` next
-   to the Travis check.
-2. Click ``Restart build``
-
-If you haven’t already signed into Travis with GitHub it will prompt you
-to do this the first time you try this, and it may take a few minutes to
-sync before you can see the page with the option to restart the build.
+7. If this is a second review, the developer should assign the issue back
+   to the developer and contact them prior to moving the issue into the
+   'Done' column. The Scrum Master can be consulted during the Stand-up.

doc/source/How-to-implement-a-command-line-utility.rst

@@ -76,13 +76,9 @@ checksums listed in
 <https://github.com/metoppv/improver/blob/master/improver_tests/acceptance/SHA256SUMS>`_.
 Changes to the checksum file in the code repository identify when
 changes are made to the code requiring corresponding changes to the data
-files. The acceptance test data files are maintained internally in a Met
-Office repository, with the directory structure being based on the CLI
-plugin names.
-
-Minimising the usage of input and output data files is desirable in the
-future, as described in issue
-`#1218 <https://github.com/metoppv/improver/issues/1218>`_.
+files. The acceptance test data files are maintained in a public repository:
+`https://github.com/metoppv/improver_test_data`, with the directory
+structure being based on the CLI plugin names.
 
 Use of checksums
 ~~~~~~~~~~~~~~~~

doc/source/Running-at-your-site.rst

@@ -131,24 +131,35 @@ tests are quick to run. Unit tests are run as part of the test suite on
 
    # Run unit tests via improver-tests wrapper
    bin/improver-tests unit
+   bin/improver-tests --help # Prints out the help information
    # Use pytest directly with marker to run only the unit tests
    pytest -m 'not acc'
+   # To run a particular function within a unit test, you can use the :: notation
+   pytest -m improver_tests/test_unit_test.py::Test_function
 
 The CLI (command line interface) acceptance tests use known good output
-(KGO) files on disk for validating that the behaviour is as expected.
-These data files are large, so the acceptance tests are not run on
-Github actions. Contact a `Met Office IMPROVER
-contributor <https://github.com/metoppv/improver/commits/master>`_ to
-arrange for a copy of the acceptance test input and output files.
+(KGO) files for validating that the behaviour is as expected. This data
+can be found in the `improver_test_data` open source repository on GitHub.
 
 The path to the acceptance test data is set using the
 ``IMPROVER_ACC_TEST_DIR`` environment variable. Acceptance tests will be
 skipped if this environment variable is not defined.
+To run the acceptance tests you can use the following:
 
 .. code:: bash
 
-   export IMPROVER_ACC_TEST_DIR=/path/to/acceptance/data
+   export IMPROVER_ACC_TEST_DIR=/path/to/acceptance/data/repo
    # Use pytest marker to run only the acceptance tests
    pytest -m acc
    # Acceptance tests can be run significantly faster in parallel using the pytest-xdist plugin
    pytest -n 8
+   # An example of running just one particular acceptance test
+   pytest -v -s -m acc -k test_cli_name.py
+
+To run all tests together at once, the following command can be input
+
+.. code:: bash
+
+   bin/improver-tests # runs all tests
+   bin/improver-tests unit # runs unit tests
+   bin/improver-tests --help # Prints out the help information

doc/source/Ticket-Creation-and-Definition-of-Ready.rst

@@ -9,10 +9,10 @@ User stories
 
 When creating new tickets you should follow the template provided:
 
-   As a X I want Y so that Z
-
    Related issues: #I, #J
 
+   As a W I want X so that Y. It is done when Z.
+
    Optional extra information text goes here
 
    Acceptance criteria:
@@ -49,15 +49,9 @@ Type
 * Investigative: A more open-ended investigation, into a technical or
   scientific development, which may or may not lead to future work to implement
   the results of the investigation.
-* Maintenance: Improvements to the design, structuring and/or behaviour of a
-  function. Can be viewed as the removal of minor technical debt to improve the
-  supportability and maintainability.
-* Optimisation: An optimisation task mostly focussed on improving the
-  timeliness of processing.
-* Review: A review of the current status of a piece of functionality is
-  required, ahead of starting more work.
 * Technical Debt: Previous issues have resulted in the accumulation of
   technical debt. These issues do the groundwork to remove the technical debt.
+  This also covers code maintenance and optimisation.
 
 Inactive:
 ~~~~~~~~~
@@ -71,7 +65,7 @@ Inactive:
   reasons, it is not possible to fix the problem described in this
   issue, or we are acknowledging that we won’t fix this issue as it’s
   low priority, and alternative solutions may need to be investigated.
-* On Hold: Work started on this issue, however, the priority of this
+* Paused: Work started on this issue, however, the priority of this
   issue decreased, and is now On Hold, potentially until a firmer
   requirement can be established.