Skip to content

Commit

Permalink
Responding to review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Katharine Hurst committed Nov 11, 2024
1 parent 5618e03 commit 28ff201
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 15 deletions.
12 changes: 5 additions & 7 deletions doc/source/Code-Style-Guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ 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.
* Avoid adding functionality to CLIs beyond calling the relevant plugin,
all other functionality should be added to the plugin itself.

Pull requests
~~~~~~~~~~~~~
Expand Down Expand Up @@ -537,10 +536,9 @@ 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.
As mentioned above, it is important to ensure that no functionality
other than calling the plugin exists within the CLI layer.
Any checks on the data or input requirements 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
Expand Down
13 changes: 7 additions & 6 deletions doc/source/Guidance-for-Reviewing-Code.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ As a Reviewer:
e. **The criteria within the Definition of Done has been satisfied.**
f. **Ensure their testing is documented on the issue.**

4. Reviewers should post comments to the pull request to show that
3. 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:
4. Things to consider when reading through the code are:

a. **Naming conventions and coding style**

Expand All @@ -73,21 +73,22 @@ As a Reviewer:

d. **Functionality**

* Does the code do what is supposed to?
* Does the code do what it's supposed to?
* Are errors handled appropriately?
* Is the code written to be run efficiently?
* Is the code efficient with respect to processing time and
memory requirements?

e. **Test coverage**

* Do the tests provided cover the expected situations?
* Are the tests understandable?
* Have edge cases been considered?

6. If this is a first review, the developer should then move the issue
5. If this is a first review, the developer should then move the issue
into 'Second Review' and a second reviewer should ensure that the
criteria listed above have been met. The Scrum Master can be
consulted, if necessary.

7. If this is a second review, the developer should assign the issue back
6. 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.
2 changes: 0 additions & 2 deletions doc/source/Running-at-your-site.rst
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,3 @@ 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

0 comments on commit 28ff201

Please sign in to comment.