Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions source/Development/documentation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 15 additions & 7 deletions source/Development/planning_your_change.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
------------------------------------

Expand Down
4 changes: 4 additions & 0 deletions source/Reviewers/codereview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------------------

Expand Down
5 changes: 5 additions & 0 deletions source/Reviewers/scitechreview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------------------

Expand Down
37 changes: 30 additions & 7 deletions source/WorkingPractices/gh_dev_init.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,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::

Expand Down Expand Up @@ -195,7 +216,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::

Expand Down
9 changes: 5 additions & 4 deletions source/WorkingPractices/pull_requests.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
----------------------
Expand Down