Misc updates (#470)

Author: jennyhickson

source/Development/documentation.rst

@@ -35,8 +35,9 @@ alongside the code. This is then compiled using Sphinx into the webpages above.
 
     The UM documentation papers are written in LaTeX and stored in a separate
     repository. Guidelines for editing the UM documentation papers are available
-    here `https://code.metoffice.gov.uk/trac/um/wiki/WorkingPractices/Documentation
-    /UpdatingUMDPs`_.
+    `here <umdp>`_
+
+.._ umdp: https://code.metoffice.gov.uk/trac/um/wiki/WorkingPractices/Documentation/UpdatingUMDPs
 
 LFRic Apps and Core also use doxygen to document the code and all changes
 should include appropriate doxygen changes to go with them. Doxygen guidelines

source/Development/planning_your_change.rst

@@ -33,13 +33,18 @@ The following are some general hints and tips in planning code changes successfu
 
 General Considerations
 ----------------------
+**Smaller is better.** Submit a separate pull request per bug fix or feature.
+Avoid refactoring or reformating code that is not related to your change.
+Multiple small pull requests are easier to review and more likely to be accepted
+promptly.
 
-**How complex is your change likely to be?** (e.g. roughly how many subroutines
-or lines of code do you expect to alter or add?) This is an important
-consideration as the more complex a change is, the more time will be required
-in development, the more code owners will need to approve it and so forth. If a
-change is overly complex, the developer should consider breaking it up into
-smaller, more manageable and, where possible, "self contained" tickets.
+**Consider the complexity of your change.** For larger or more complex changes,
+start by opening an issue and discussing your approach with the relevant people.
+This helps avoid unnecessary work and ensures alignment with project goals.
+
+**Prioritise clarity over cleverness.** Code is read more often than it is
+written, so make it easy to understand and maintain. If the logic is not
+immediately obvious then include comments to explain your reasoning.
 
 **How does your proposed change fit in with the structure of the model?** Try
 and make your code changes in-scope and no larger than they need to be. If you
@@ -54,7 +59,7 @@ aware of these.
 * `UMDP3 (UM and JULES FORTRAN)
   <https://code.metoffice.gov.uk/doc/um/latest/umdp.html#003>`__
 * `LFRic Coding Styles
-  <https://code.metoffice.gov.uk/trac/lfric/wiki/LFRicTechnical/CodingStandards>`__
+  <https://metoffice.github.io/lfric_core/how_to_contribute/index.html#how-to-contribute-index>`__
 * `PEP 8 (Python) <https://legacy.python.org/dev/peps/pep-0008/>`__
 
 **Who will SciTech review the change?** This is a useful consideration as not
@@ -78,6 +83,9 @@ linked tickets. See :ref:`multirepo` for further details.
 idea **not** to re-invent the wheel or have code duplication! Speaking to code
 owners of the appropriate sections can help in this instance.
 
+**How will your change be tested?** Include unit or integration tests, and update
+any example or demo repositories to exercise new functionality.
+
 Specific Tips for Scientific changes
 ------------------------------------
 

source/Reviewers/codereview.rst

@@ -20,6 +20,10 @@ Fundamentally this review is to ensure that the change is well thought-out and
 that no system aspects have been missed. The review should be an active one
 and should question anything that is poorly coded.
 
+Focus on the code, not the contributor; providing constructive, respectful and
+actionable feedback. Critique the implementation, not the individual and always
+explain the reasoning behind your suggestions.
+
 Reviewer responsibilities and checkpoints
 -----------------------------------------
 

source/Reviewers/scitechreview.rst

@@ -16,6 +16,11 @@ Purpose of the review
 The purpose of code review is to ensure that the code does the job it says it
 performs, is standards compliant and well documented.
 
+Focus on the code, not the contributor; providing constructive, respectful and
+actionable feedback. Critique the implementation, not the individual and always
+explain the reasoning behind your suggestions.
+
+
 Reviewer responsibilities and checkpoints
 -----------------------------------------
 

source/WorkingPractices/gh_dev_init.rst

@@ -12,19 +12,40 @@ created a fork (:ref:`forking`).
 Create an Issue
 ---------------
 
+An issue in github can be used to document a new feature, bug, or other problem
+in the codebase. There are issue templates provided to help capture all the
+relevant information in each case.
+
+They are also the main place to document the development process of a change.
+Sub-issues can be created if a large piece of work wants breaking down into
+smaller sections. If you are working on an issue, then assign yourself to it so
+that others know that you are working on it. Issues are created in the upstream
+repository.
+
+.. admonition:: Before Opening an Issue
+
+    **Are you using the latest version?** If not, please update to the latest
+    stable release and verify the issue persists.
+
+    **Is this a security issue?** Please do not file a public issue for
+    security vulnerabilities, but get in touch with the :ref:`SSD team <ssd>`
+    directly.
+
+    **Is this a duplicate?**  Add a comment or an emoji reaction to the original
+    issue rather than opening a new one.
+
+    **Is this a support request?** Questions should be asked via the
+    `simulation-systems discussion boards`_ where we'll be happy to try and help.
+
+.. _simulation-systems discussion boards: https://github.com/MetOffice/simulation-systems/discussions
+
 .. important::
 
     It is not guaranteed that opening an issue will result in action or even
     visibility by the relevant maintainers or code owners. If you think a team
     or individual should be aware of an issue, then contact them directly in
     addition to opening an issue.
 
-An issue in github can be used to document a problem in the codebase or as
-somewhere to document the development process for a new feature. Sub-issues
-can also be created if a large piece of work wants breaking down into smaller
-sections. If you are working on an issue, then assign yourself to it so that
-others know that you are working on it. Issues are created in the upstream
-repository.
 
 .. tab-set::
 
@@ -198,7 +219,9 @@ And then commit the change,
     information see the relevant man page, ``man git add``.
 
 Finally, you may want to push any commits stored in your local clone back to
-the remote source.
+the remote source. This allows you to share code with someone else during the
+development process, or prepare to open a pull request once you're ready for a
+review.
 
 .. code-block::
 

source/WorkingPractices/pull_requests.rst

@@ -17,12 +17,10 @@ Trivial pull requests are an exception and do not require a SciTech review.
 
 :ref:`Linked pull requests <linked>` will move through the review states together.
 
-.. tip::
-
-  Some things to consider when getting ready for review:
+.. admonition:: Getting Ready for Review
 
   * If your development changes answers then make sure you have followed the
-    steps on :ref:`preparing a KGO ticket for review.<kgo>`
+    steps on :ref:`preparing a KGO pull request for review.<kgo>`
 
   * Get in touch with your SciTech Reviewer before you feel ready for review.
     They will have valuable insights into the code and, particularly for larger
@@ -40,6 +38,9 @@ Trivial pull requests are an exception and do not require a SciTech review.
     close that part of the review. The developer should **not** resolve
     conversations themselves.
 
+  * Feedback on code is not a refection of personal ability. All code can be
+    improved and reviews are an opportunity for shared learning and collaboration.
+
 
 Opening a Pull Request
 ----------------------