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
43 changes: 42 additions & 1 deletion source/Reviewers/curaterelease.rst
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,55 @@ developed the release branch and the other will review it (**developer** and
* The **reviewer** can now ``merge`` the second PR.
* The admin **must** now revert the 2 settings above in ``main`` and remove the
exception for ``stable`` in the ``prevent updates`` ruleset from earlier.
* :ref:`Tag <tagging>` the release.

Hotfix Release
--------------

This section describes the process of applying a hotfix to the most recent
release. This section is relevant for all repositories with a stable and main
branch setup. The hotfix process will be completed by 2 people with commit
privilege to the relevant repository, at least one of whom must be an ``admin``.
One will have developed the hotfix branch and the other will review it
(**developer** and **reviewer** below).

* An admin will need to enable updates to the ``stable`` branch by adding it as
an exception to the target branches in the ``prevent updates`` ruleset. They
should also enable ``squash`` as a merge option in the ``stable`` ruleset.
* The **developer** will make the hotfix change, making sure the branch has been
created from ``stable``. Open a PR for this change targetting ``stable`` and
get it reviewed and committed. The **reviewer** should squash this change into
``stable``.
* The admin should now update the ``main`` ruleset:

* Disable ``Require linear history``.
* Set ``merge`` as an allowed merge strategy and disable ``squash``.
* Uncheck the ``Require branches to be up to date`` box.

* The **developer** will then create another new PR, to merge the
``stable`` branch into ``main``.

.. image:: images/gh_screenshots/stable_main_light.png
:class: only-light border

.. image:: images/gh_screenshots/stable_main_dark.png
:class: only-dark border

* The **reviewer** can now ``merge`` the second PR.
* The admin **must** now revert the 3 settings above in ``main`` and remove the
exception for ``stable`` in the ``prevent updates`` ruleset from earlier.
* :ref:`Tag <tagging>` the hotfix.

.. _tagging:

Tags and Releases
^^^^^^^^^^^^^^^^^

* All repositories will be tagged with the Simulation Systems release tag in the
format YYYY.MM.X.
* In an upto-date clone of the repository:

* In an upto-date clone of the repository, switch to the commit that you would
like to create a tag from (probably ``git switch stable``):

.. code-block:: shell

Expand Down
55 changes: 34 additions & 21 deletions source/Reviewers/howtocommit.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ of these steps outlined below.
Before You Start:
* Is anyone else committing?

* `Main Status`_ is used to coordinate ``main`` commits for all
* `Repository Status`_ is used to coordinate ``main`` commits for all
projects.

* This operates on a first-come-first-served queing system.
* To join the queue use the ``Add Item`` button.
* Do not move yourself up the queue unless agreed with others.

* Simple, not conflicting commits can be done in parallel if
reviewers all agree.
* Changes with KGO or Macros usually require sole access to ``main``.
Expand Down Expand Up @@ -66,12 +71,17 @@ there are more complicated conflicts ask the developer to solve them
themselves. If there are conflicts in versions.py then see the details in the
macro section below.

Then switch to the up to date branch, e.g.
Then clone the up to date branch. There are many ways to do this, eg.

.. code-block:: shell

gh repo clone <developer>/<fork_name> <clone_name> -- -b <branch_name>

# OR

cd /path/to/upstream/clone
gh pr checkout <pr_number>

2. Macros (if required)
-----------------------

Expand Down Expand Up @@ -371,24 +381,27 @@ KGO differently*

cylc vip -z group=all -n <name/of/suite> ./rose-stem

#. You will need access to both a clone with the branch active, and
up to date with ``main``, and a clone of the `SimSys_Scripts github
repo <https://github.com/MetOffice/SimSys_Scripts>`__ (one is
available in $UMDIR). Run the script
``kgo_updates/meto_update_kgo.sh`` which is located in
SimSys_Scripts.
#. You will need access to both:

* The PR branch, up to date with ``main`` and cloned locally. You
likely already have this.
* A clone of the `SimSys_Scripts github repo
<https://github.com/MetOffice/SimSys_Scripts>`__ (one is available
in $UMDIR). Run the script ``kgo_updates/meto_update_kgo.sh`` which
is located in SimSys_Scripts.

#. The script will ask you to enter some details regarding the PR.
* Platforms: enter each platform which has a kgo change, lower case
and space seperated, e.g. `azspice ex1a`
* If running on the EX's it will ask for the host you ran on - this
can be found from Cylc Review.
* Path to your local clone - the script will check this exists and
will fail if it can't be found.
* KGO directory: this will default to vnXX.X_tYYYY where XX.X is
the version number and YYYY is the PR number.
* There are further prompts to the user through the script - in
particular to check the shell script produced.

* Platforms: enter each platform which has a kgo change, lower case
and space seperated, e.g. `azspice ex1a`
* If running on the EX's it will ask for the host you ran on - this
can be found from Cylc Review.
* Path to your local clone - the script will check this exists and
will fail if it can't be found.
* KGO directory: this will default to vnXX.X_tYYYY where XX.X is
the version number and YYYY is the PR number.
* There are further prompts to the user through the script - in
particular to check the shell script produced.

#. If running on EX's the script will ask whether to rsync UM files or
lfricinputs files to the other EX hosts. Select the appropriate
Expand Down Expand Up @@ -620,13 +633,13 @@ the PR via the GitHub interface,
6. Close
--------

Nightly testing results are usually checked with a status posted on `Main
Nightly testing results are usually checked with a status posted on `Repository
Status`_. If this hasn't been done then :ref:`check the nightly results
<nightlytesting>`.

If something is broken:

* Announce to the team and on `Main Status`_.
* Announce to the team and on the `Repository Status`_.
* There are a few possibilities for how to proceed,

* If the fix is obvious and trivial then create a quick PR on a branch from
Expand Down Expand Up @@ -691,4 +704,4 @@ If something is broken:
* You can then access other machines as the admin user via ``ssh -Y
<HOSTNAME>``.

.. _Main Status: https://code.metoffice.gov.uk/trac/lfric_apps/wiki/TrunkStatus
.. _Repository Status: https://wwwspice/~umadmin/simsys_commit_status/index.html
108 changes: 48 additions & 60 deletions source/WorkingPractices/reviews.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,31 @@ Selecting Reviewers
-------------------

There is a space in the pull request template to list the GitHub user ID of the
scitech and code reviewers. These can also be filled into the appropriate
scitech and code reviewers - these will also be filled into the appropriate
:ref:`project spaces <review_project>`.

.. admonition:: Requesting a Review

Review requests are handled in the ``Reviewers`` pane on the right hand
side of a pull request.

In Simulation Systems repositories, SciTech and Code Reviewers will be
automatically requested by a workflow, based on entries in the Pull Request
Template.

To request a review manually, select the cog, and then search for the person
you wish to review by either name or GitHub user ID.

.. image:: images/gh_screenshots/review_cog_light.png
:class: only-light border

.. image:: images/gh_screenshots/review_cog_dark.png
:class: only-dark border

Code owners in some repositories will automatically be added to this
reviewers section based on the files being changed.


SciTech Review
^^^^^^^^^^^^^^

Expand All @@ -25,8 +47,9 @@ owner(s) for the area affected. If they don't want to then they may have
suggestions for other suitable reviewers or you can approach anyone who would
have good insight into the changes made.

Once you have found a reviewer add their GitHub user ID to the pull request
description and request their review.
Once you have found a reviewer and are ready for review, add their GitHub user
ID to the pull request description. Their review will automatically be
requested by the :ref:`review project workflow <review_project>`.

Guidance for the SciTech reviewer can be found on the
:ref:`SciTech review page <scitech_review>`.
Expand All @@ -42,30 +65,13 @@ leave a comment for ``@MetOffice/ssdteam`` on the pull request.

The assigned person will be listed in the pull request description and
:ref:`Review Tracking project <review_project>`. Once the SciTech review has
been completed either the developer or SciTech reviewer should request the
review of the assigned Code Reviewer.
been completed, the Code Reviewer will be automatically requested (unless they
are already requested as a Code Owner - in this case you may want to tag them to
make them aware).

Guidance for the Code reviewer can be found on the
:ref:`Code review page <code_review>`.

.. admonition:: Requesting a Review

Review requests are handled in the ``Reviewers`` pane on the right hand
side of a pull request.

Select the cog, and then search for the person you wish to review by
either name or GitHub user ID.

.. image:: images/gh_screenshots/review_cog_light.png
:class: only-light border

.. image:: images/gh_screenshots/review_cog_dark.png
:class: only-dark border

Code owners in some repositories will automatically be added to this
reviewers section based on the files being changed.


.. _review_project:

Simulation Systems Review Tracker
Expand All @@ -82,45 +88,27 @@ sidebar of the pull request:
:class: only-dark border

The project is used to give pull requests a status that distinguishes between
the different review states, and to monitor who is doing the reviews.

Some states are achieved automatically, some require changing manually:

* When the developer feels a PR is ready for the SciTech or Code Reviewer to
look at (either initially, or after changes have been made) the state should
be **manually** changed to ``SciTech Review`` or ``Code Review`` as
appropriate.

* When the SciTech Review has been completed the state should be **manually**
changed to Code Review.

* When the Code Review has been completed the state should be **manually**
changed to Approved.

Automatic changes include:

* When changes are requested by a reviewer the state becomes ``Changes Requested``
* When the pull request has been merged, or otherwise closed, the state becomes
``Done``

.. important::
Changing the project status **does not** notify the reviewer. To do this:

* When the SciTech Review has been completed you should add the assigned Code
Reviewer to the list of reviewers. This will notify them that their review is
required.

* If a reviewer has requested changes then you can alert them that you are
ready for another review by using GitHubs ``rerequest review`` option;
selecting the circling arrows to the right of the reviewers name.

.. image:: images/gh_screenshots/rerequest_light.png
:class: only-light border
the different review states, and to monitor who is doing the reviews. The state
and fields of the project are automatically updated by github actions.

* The developer should tag the SciTech Reviewer in the Pull Request template
when you are ready for review. This will request a review from them and update
the state to SciTech Review.
* The SSD team will add a Code Reviewer. When the Scitech Review has been
completed a review from the Code Reviewer will automatically be requested, and
the project state updated. If the Pull Request is trivial, you will need to
manually request a review from the Code Reviewer.
* As reviews are completed the state will move to ``Changes Requested`` and
``Approved`` as appropriate. Re-request a review once any changes have been
made.

.. image:: images/gh_screenshots/rerequest_light.png
:class: only-light border

.. image:: images/gh_screenshots/rerequest_dark.png
:class: only-dark border
.. image:: images/gh_screenshots/rerequest_dark.png
:class: only-dark border

* You can ``@username`` in any comment to draw that persons attention to the
* You can ``@username`` in any comment to draw that persons attention to the
pull request.


Expand Down