From dba857f1a6ef119576b0b8be3be5d36391459abe Mon Sep 17 00:00:00 2001 From: James Bruten Date: Thu, 23 Oct 2025 13:32:21 +0100 Subject: [PATCH 01/20] trac and ticket keyword blat --- .../Diagnostics/um_stashmaster.rst | 5 +- source/Development/TestSuites/jules.rst | 2 +- source/Development/TestSuites/lfric_apps.rst | 2 +- .../TestSuites/multi-repo_testing.rst | 9 ++- source/Development/TestSuites/ukca.rst | 2 +- source/Development/TestSuites/um.rst | 2 +- source/Development/inputs.rst | 4 +- source/Development/kgo.rst | 8 +-- source/Development/macros.rst | 15 +++-- source/Development/metadata_guidance.rst | 2 +- source/Development/planning_your_change.rst | 8 +-- source/Development/temp_logicals.rst | 4 +- source/FurtherDetails/coding_style.rst | 4 +- source/FurtherDetails/dos_donts.rst | 43 ++++--------- source/FurtherDetails/glossary.rst | 34 ++++------- source/FurtherDetails/who.rst | 4 +- source/Reviewers/committinglinkedtickets.rst | 32 +++++----- source/Reviewers/curaterelease.rst | 10 ++-- source/Reviewers/howtocommit.rst | 60 +++++++++---------- source/Reviewers/index.rst | 8 +-- source/Reviewers/releases/jules_release.rst | 6 +- .../Reviewers/releases/lfric_apps_release.rst | 6 +- source/Reviewers/releases/mule_release.rst | 2 +- source/Reviewers/releases/partner_testing.rst | 2 +- source/Reviewers/releases/software_stack.rst | 2 +- source/Reviewers/releases/um_main_release.rst | 2 +- source/WorkingPractices/final_steps.rst | 35 ++++------- source/WorkingPractices/gh_dev_init.rst | 2 +- source/WorkingPractices/working_practices.rst | 6 +- 29 files changed, 137 insertions(+), 184 deletions(-) diff --git a/source/Development/Diagnostics/um_stashmaster.rst b/source/Development/Diagnostics/um_stashmaster.rst index e1a711c2..ee48307c 100644 --- a/source/Development/Diagnostics/um_stashmaster.rst +++ b/source/Development/Diagnostics/um_stashmaster.rst @@ -18,9 +18,6 @@ The UM's ``STASHmaster_A`` and associated help text file The following principles apply when altering the STASHmaster: -.. - JW suggest need to include STASH entry guidance here. Maybe an issue for this would be useful? - * If you add a new diagnostic to the ``STASHmaster_A`` file then you **must** also add to the stash master help text in :ref:`stashmaster-meta`. * If you are altering the stashmaster, this may be referred to the FFPP @@ -29,7 +26,7 @@ The following principles apply when altering the STASHmaster: * If your change has new stash items or changed/added attributes as an option code, versions mask etc., then first you have to get them reserved and recorded (published) on the reservation web page STASH/ReservedCodes -* Note that every reservation should be linked to a ticket with the correct +* Note that every reservation should be linked to an Issue with the correct explanation and a milestone. This rule applies to all stash related tables placed on this page. * Although reservations could be some kind of self-service, contact the section diff --git a/source/Development/TestSuites/jules.rst b/source/Development/TestSuites/jules.rst index 16e1742c..fd597f45 100644 --- a/source/Development/TestSuites/jules.rst +++ b/source/Development/TestSuites/jules.rst @@ -44,7 +44,7 @@ on Met Office systems. +----------------------+----------------------------------------------------------+ | all | The complete test suite. This is run automatically | | | every night and monitored by the SSD team. All | -| | :ref:`KGO ` changing tickets need to run this group.| +| | :ref:`KGO ` changing PRs need to run this group. | +----------------------+----------------------------------------------------------+ +----------------------+----------------------------------------------------------+ | loobos | A set of tests to exercise these science areas. | diff --git a/source/Development/TestSuites/lfric_apps.rst b/source/Development/TestSuites/lfric_apps.rst index 37f87ed7..f45acafa 100644 --- a/source/Development/TestSuites/lfric_apps.rst +++ b/source/Development/TestSuites/lfric_apps.rst @@ -50,7 +50,7 @@ groups, and that you can specify more than one at once, e.g. | all | The complete test suite, including all longer runs and | | | less commonly used configs. This is run automatically | | | every week and monitored by the SSD team. All | -| | :ref:`KGO ` changing tickets need to run this group.| +| | :ref:`KGO ` changing PRs need to run this group. | +--------------------+----------------------------------------------------------+ +--------------------+----------------------------------------------------------+ | build | Compile tasks for all applications and science areas | diff --git a/source/Development/TestSuites/multi-repo_testing.rst b/source/Development/TestSuites/multi-repo_testing.rst index 857c41d3..209a847d 100644 --- a/source/Development/TestSuites/multi-repo_testing.rst +++ b/source/Development/TestSuites/multi-repo_testing.rst @@ -4,11 +4,10 @@ Multi-Repository Testing ======================== Multi-repository changes are expected to pass the regression tests for all the -repositories involved. To carry out the tests involved in a linked ticket it -can be helpful to refer to the :ref:`repository figure `; testing -both child and parent repositories as needed. Further details of how testing -in each repository is handled can be found on the :ref:`Testing -page`. +repositories involved. To carry out the tests involved in a linked PR it can be +helpful to refer to the :ref:`repository figure `; testing both child +and parent repositories as needed. Further details of how testing in each +repository is handled can be found on the :ref:`Testing page`. All Simulation Systems repositories containing a test suite will also contain a ``dependencies.yaml`` file in the top directory of the repository. This file diff --git a/source/Development/TestSuites/ukca.rst b/source/Development/TestSuites/ukca.rst index 132fea76..88bacf7a 100644 --- a/source/Development/TestSuites/ukca.rst +++ b/source/Development/TestSuites/ukca.rst @@ -2,7 +2,7 @@ Testing UKCA ============ Changes in UKCA that touch `src/science` or `src/control/core` must be tested -with both the UM and LFRic by following the :ref:`linked tickets guidance +with both the UM and LFRic by following the :ref:`linked PRs guidance `. For further guidance on testing and working with UKCA, including standard diff --git a/source/Development/TestSuites/um.rst b/source/Development/TestSuites/um.rst index f431373f..691ef016 100644 --- a/source/Development/TestSuites/um.rst +++ b/source/Development/TestSuites/um.rst @@ -56,7 +56,7 @@ groups, and that you can specify more than one at once, e.g. | all | The complete test suite, including all longer runs and | | | less commonly used utilites. This is run automatically | | | every week and monitored by the SSD team. All | -| | :ref:`KGO ` changing tickets need to run this group.| +| | :ref:`KGO ` changing PRs need to run this group. | +--------------------+----------------------------------------------------------+ +--------------------+----------------------------------------------------------+ | rigorous_compile | A build-only group that will sense-check the code for a | diff --git a/source/Development/inputs.rst b/source/Development/inputs.rst index f712f066..5aa12c31 100644 --- a/source/Development/inputs.rst +++ b/source/Development/inputs.rst @@ -102,7 +102,7 @@ whether an upgrade macro is necessary. cd rose-meta rose edit -C ../rose-stem/app/APP-NAME --no-warn version - If you have a linked LFRic Core or Jules ticket with metadata changes, you + If you have a linked LFRic Core or Jules PR with metadata changes, you can load their metadata by adding ``-M /path/to/working_copy/rose-meta`` to the ``rose-edit`` command. @@ -123,7 +123,7 @@ Adding a new metadata section requires: (see existing ones for examples) The ``vnX.Y`` and ``HEAD`` metadata should be identical for this initial -ticket, other than any import statements, which should point at vnX.Y or HEAD +PR, other than any import statements, which should point at vnX.Y or HEAD respectively. Other ``vnX.Y`` and ``versionAB_CD.py`` files shouldn't be modified or added (these are a snapshot of the metadata at a release). diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index 5a36bc6e..be387035 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -13,7 +13,7 @@ may also be required to simply add a new job to the test suite or to port the rose stem suite to new HPC architecture. **LFRic** KGO checksums are stored in the repository. As such with LFRic -tickets the expectation is that you, as the developer, will include updated +PRs the expectation is that you, as the developer, will include updated KGO files as part of your branch. **UM and JULES** KGO output files are stored outside of the repository. Access @@ -25,16 +25,16 @@ the commit process. KGO Update Process ------------------ -Getting the process right for KGO changing tickets significantly helps get such +Getting the process right for KGO changing PRs significantly helps get such changes onto the trunk. When preparing your change for review: 1. Run the ``all`` rose-stem group in order to make sure that all changes to answers have been found. * Include the :ref:`trac.log ` output from this testing in your - ticket summary. + PR summary. -2. Add the ``kgo`` keyword to your ticket. +2. Add the ``kgo`` label to your PR. .. tab-set:: diff --git a/source/Development/macros.rst b/source/Development/macros.rst index 2a439c4b..1c75a8f6 100644 --- a/source/Development/macros.rst +++ b/source/Development/macros.rst @@ -54,7 +54,9 @@ Within the file a blank upgrade macro will typically look like this: Note: The BEFORE_TAG should match the AFTER_TAG of the previous macro in the chain. So if this is not the first macro since the release then the BEFORE_TAG -will be the version number with an added ticket number as well. For example: +will be the version number with an added ticket number as well. For github +developments, the ticket number can be either an Issue or PR number. For +example: .. code-block:: python @@ -137,8 +139,10 @@ It is expected that all metadata changes in LFRic Core will require change to the rose-apps in LFRic Apps, but changes to Apps must not affect Core. Therefore, the apply_macros script requires a working copy of LFRic Apps to work, but will source it's own copy of Core if required. If your only changes -are to LFRic Core metadata, then you will require a linked LFRic Apps ticket -and test branch, but potentially not a development branch. +are to LFRic Core metadata, then you will require a linked LFRic Apps PR and +test branch, but potentially not a development branch. In order to avoid +potential ticket number clashes between LFRic Core and LFRic Apps, we recommend +using LFRic Apps Issue/PR number for the macro tag. .. important:: @@ -190,5 +194,6 @@ Apps working copy). Core and Jules will default to reading the The ``vnXX.Y_tTTTT`` option must match the After Tag of your upgrade macro. When setting this, the version is the last released version of LFRic Apps. If -it's a linked Apps-Core ticket, then set the ticket number as the one where -the most metadata changes are being made. +it's a linked Apps-Core PR, then set the ticket number based on the Apps +Issue or PR. This avoids potential ticket number clashes between the +repositories. diff --git a/source/Development/metadata_guidance.rst b/source/Development/metadata_guidance.rst index 2cd9150b..4a791ceb 100644 --- a/source/Development/metadata_guidance.rst +++ b/source/Development/metadata_guidance.rst @@ -63,7 +63,7 @@ more details including what should be in `jules-shared `__ and in `jules-standalone, jules-lfric or um-atmos `__. -When developing shared JULES metadata, you will need :ref:`linked tickets +When developing shared JULES metadata, you will need :ref:`linked PRs `. The metadata migration is currently dictated by LFRic porting of science, although the ultimate aim is to have a single source of truth. diff --git a/source/Development/planning_your_change.rst b/source/Development/planning_your_change.rst index b0f42447..f1982826 100644 --- a/source/Development/planning_your_change.rst +++ b/source/Development/planning_your_change.rst @@ -64,7 +64,7 @@ aware of these. **Who will SciTech review the change?** This is a useful consideration as not everyone who uses the repository has the knowledge or experience to review -every ticket that is being developed. Get in touch with your SciTech reviewer +every PR that is being developed. Get in touch with your SciTech reviewer early in the process as they will have valuable insights that can help to shape your change. @@ -77,7 +77,7 @@ approvals to take place. **Is the code you need to alter on a single repository or is it spread over multiple repositories?** If it's over multiple repositories you need to use -linked tickets. See :ref:`multirepo` for further details. +linked PRs. See :ref:`multirepo` for further details. **Does similar code functionality already exist in the model?** It's a good idea **not** to re-invent the wheel or have code duplication! Speaking to code @@ -98,7 +98,7 @@ upgrade macro to include the switch into the upgraded configuration. vital to make sure your code changes work when switched **on** and give the same answer when the code is run over different processor configurations. Producing a quick plot or plots to show the impact of your code and including -them on your ticket can aid your SciTech reviewer in showing that your code +them on your PR can aid your SciTech reviewer in showing that your code works properly. **Does the change need any new diagnostics to make sense of the code?** Many @@ -124,6 +124,6 @@ intends to optimise code, be prepared to provide evidence of how things have improved. .. - Comment: Are there any more that can be thought of? These tickets will + Comment: Are there any more that can be thought of? These PRs will mostly be done by experienced developers and usually inside the Met Office. diff --git a/source/Development/temp_logicals.rst b/source/Development/temp_logicals.rst index d9db8dc7..684a4fbb 100644 --- a/source/Development/temp_logicals.rst +++ b/source/Development/temp_logicals.rst @@ -156,7 +156,7 @@ temporary logical should be removed from the code base. .. important:: - Prior to a ticket containing a temporary logical being committed to the - trunk, the developers is expected to open a new ticket which removes the + Prior to a PR containing a temporary logical being committed to the + trunk, the developers is expected to open a new Issue which removes the logical after a fixed period. This acts as an memory aid that the logical needs to be removed in due course. diff --git a/source/FurtherDetails/coding_style.rst b/source/FurtherDetails/coding_style.rst index 5fcb3fd5..d086d0cd 100644 --- a/source/FurtherDetails/coding_style.rst +++ b/source/FurtherDetails/coding_style.rst @@ -327,8 +327,8 @@ S2. Headers same and thus it need not be repeated in the latter. If a ``MODULE`` contains more than one subroutine then further descriptions are required. -- History comments should not be included in the header or routine code. FCM - TRAC provides the history of our codes. +- History comments should not be included in the header or routine code. Version + control provides the history of our codes. - Code author names should NOT be included explicitly within the code as they quickly become out of date and are sometimes misleading. Instead we diff --git a/source/FurtherDetails/dos_donts.rst b/source/FurtherDetails/dos_donts.rst index 2957b911..337bcc26 100644 --- a/source/FurtherDetails/dos_donts.rst +++ b/source/FurtherDetails/dos_donts.rst @@ -10,53 +10,36 @@ Please Do mitigate problems early on. This is the most common root cause of problems, sometimes years later. -**Plan** your work aimed at the trunk across single or multiple tickets: - * Ensure tickets are not too big or small. - * Coherent parts of the overall change are contained in a single ticket - * Consider using an overarching ticket to link everything together +**Plan** your work aimed across single or multiple Issues and Pull Requests: -**Document your work** using tickets, TRAC pages, :ref:`formal documentation + * Each pull request should contain a single coherent change. + * Consider using an overarching issue to link everything together + +**Document your work** using Issues and PRs pages, :ref:`formal documentation ` and code comments. These help others and your future-self understand your work. -**Meaningful names** for tickets, branches and variables. These help others and -your future-self understand your work. "My_Branch", "Fix" are not helpful. +**Meaningful names** for issues, PRs, branches and variables. These help others +and your future-self understand your work. "My_Branch", "Fix" are not helpful. **Be considerate** of other users/developers. Their skill-sets and working days may be very different to yours. All changes are visible to all users worldwide. -**Keep the ticket status up to date.** This enables the Simulation Systems and -Deployment Team to monitor the progress of your ticket and potential -conflicts. - -**Link to tickets in other MOSRS repositories**, eg jules:#1, ukca:#72 +**Link to issues/PRs in other repositories**, eg ``MetOffice/jules#1``, +``MetOffice/ukca:#72`` Please Do Not ------------- -**Do not use svn commands.** Please use `FCM -`__ for all development work. - -**Do not merge the trunk into your branch** for UM, JULES, UKCA and LFRic Apps -changes as this breaks many aspects of how TRAC and fcm work. This will cause -diffs to display incorrectly and causes database problems when merging. -Instead, please create a head of trunk branch and merge in your old branch. - -**Do not develop using head of trunk branching if not needed.** Many aspects of -the UM, JULES and UKCA workflows rely on version branching. +**Do not develop using head of main branching if not needed.** Using version +branching enables development of scientific suites, so please branch from a tag +or the ``stable`` branch wherever possible. **Licensing** - Don't add code to any project (or to any branch thereof) that has been developed under a different license without agreement from the Simulation Systems and Deployment Team. This includes lifting Fortran code or text from books. Our repositiories must not infringe copyright. -**Add or link to old code** or tickets that predate MOSRS, for example... - -* Link to tickets in old internal repositories- links will either not resolve - or be incorrect -* Add a version of the UM code older than UM 9.2 as a branch to the UM - repository - -**Request support by raising a ticket**. Newly raised tickets are not +**Request support by raising an issue**. Newly raised issues are not monitored. Use the appropriate :ref:`support` channels. diff --git a/source/FurtherDetails/glossary.rst b/source/FurtherDetails/glossary.rst index acd5f361..c531eda6 100644 --- a/source/FurtherDetails/glossary.rst +++ b/source/FurtherDetails/glossary.rst @@ -10,35 +10,21 @@ Closed Release: this way, and the rest free for changes. Code Review Deadline: - The date by which all tickets aiming to be included in a release have been + The date by which all PRs aiming to be included in a release have been moved into code review. -Colon Keyword: - The formatting pattern for certain ticket keywords. For example CR:user to - indicate that "user" will be performing the Code System Review. - CodeSys Review: - A technical review of the changes involved in the ticket, including checks + A technical review of the changes involved in the PR, including checks that code standards have been upheld and that the working practices have been followed. These reviews are generally completed by a member of the - Simulation Systems and Deployment Team. Once a review has been approved - the Code Systems Reviewer is then responsible for committing the change to - the trunk. - -.. - or the Core Capability Development Team (for LFRic only reviews). + Simulation IT Team. Once a review has been approved the Code Systems + Reviewer is then responsible for committing the change to the trunk. Development Window: The period of time between the release of one software version and the code review deadline for the following release in which new developments are accepted for review. -Further Commit: - Where a problem is found with a ticket after it has been committed, any - additional commits needed are associated with the same ticket and labelled - as a "Further Commit". Only essential and immediate fixes are treated this - way. - Known Good Output (KGO): In order to verify that the model output hasn't been modified by a set of changes the test suite contains a stored set of output as a reference. @@ -50,14 +36,14 @@ Head of Trunk: when the work being done *has* to be built on top of changes already made since the last revision. -Linked Ticket: - Work that spans two or more repositories, requiring tickets that should be +Linked PR: + Work that spans two or more repositories, requiring PRs that should be treated together and committed as a group. -Overarching Ticket: - Where a piece of work has been split into multiple sections and tickets an - extra ticket can be used to track this work. It should be closed when the - whole arc has been completed. +Overarching Issue: + Where a piece of work has been split into multiple sections an overarching + issue can be used to track this work. It should be closed when the whole arc + has been completed. Regression: A set of tests that prove that a set of code changes have not degraded the diff --git a/source/FurtherDetails/who.rst b/source/FurtherDetails/who.rst index 125d1f69..c60a41ac 100644 --- a/source/FurtherDetails/who.rst +++ b/source/FurtherDetails/who.rst @@ -79,7 +79,7 @@ of model evolution. Sci/Tech Reviewer ----------------- -A Sci/Tech reviewer is assigned for every ticket and comprises the first stage +A Sci/Tech reviewer is assigned for every PR and comprises the first stage of review that considers the change as a whole. Further details are found in the :ref:`Scitech Reviewers Guide`. In some cases, the reviewer can delegate parts of the work to another person. @@ -96,7 +96,7 @@ most relevant code owner. Code Reviewer ------------- -The Code Reviewer performs the 2nd stage of review for every ticket. Further +The Code Reviewer performs the 2nd stage of review for every PR. Further details are described in the :ref:`Code Reviewers Guide`. Reviews should be turned around on a reasonable timescale and follow the Code diff --git a/source/Reviewers/committinglinkedtickets.rst b/source/Reviewers/committinglinkedtickets.rst index 83c474ac..73bc5abe 100644 --- a/source/Reviewers/committinglinkedtickets.rst +++ b/source/Reviewers/committinglinkedtickets.rst @@ -1,16 +1,16 @@ .. _committinglinkedtickets: -Committing Linked Tickets -========================= +Committing Linked Pull Requests +=============================== -How do linked tickets work? ---------------------------- +How do linked pull requests work? +--------------------------------- -Linked tickets contain changes that all need to be committed together to work +Linked PRs contain changes that all need to be committed together to work successfully. With only some of the changes committed the repositories are considered "out of sync", with some of the test suites likely to fail as the api between the codebases is broken. For this reason, where possible, all -parts of a linked ticket should be committed on the same day to avoid nightly +parts of a linked PR should be committed on the same day to avoid nightly tests failing. :ref:`Multi-repository ` changes are nested, and the different @@ -38,18 +38,18 @@ the key places where these overlap. 3. Install KGO files for all repositories requiring them - 4. Commit the tickets as described below. + 4. Commit the PRs as described below. .. _testinglinked: -Testing linked tickets ----------------------- +Testing linked Pulls Requests +----------------------------- -With the branches from all the tickets merged into a working copy of their +With the branches from all the PRs merged into a working copy of their respective Head of Trunk these can all be used together to test the change. -Details for testing multi-repository tickets are included on the +Details for testing multi-repository PRs are included on the :ref:`Working with Multiple Repositories page`. **In summary:** @@ -81,7 +81,7 @@ to update the source being used. .. tip:: - If some of the changes in this set of tickets have already been committed + If some of the changes in this set of PRs have already been committed then see steps 2 and 4 below on how to include those changes in your testing. This is instead of the steps described above. @@ -91,8 +91,8 @@ to update the source being used. .. _committinglinked: -Committing linked tickets -------------------------- +Committing linked Pull Requests +------------------------------- Once you are happy with all your testing then the commit sequence is as follows: @@ -108,8 +108,8 @@ follows: * Ensure the ``source`` entry points at the MetOffice ssh url. * Modify ``ref`` entry for all updated repositories points to the full hash for the relevant commit. - * e.g. If a JULES ticket has been committed with hash starting abc123 - and a UKCA ticket starting at 456def, the UM dependencies file will + * e.g. If a JULES PR has been committed with hash starting abc123 + and a UKCA PR starting at 456def, the UM dependencies file will have these entries (amongst others): .. code-block:: yaml diff --git a/source/Reviewers/curaterelease.rst b/source/Reviewers/curaterelease.rst index acbf2f54..b570b01f 100644 --- a/source/Reviewers/curaterelease.rst +++ b/source/Reviewers/curaterelease.rst @@ -116,21 +116,21 @@ Main Release ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ **Dependencies** -Partner Testing, All Jules tickets committed +Partner Testing, All Jules PRs committed :ref:`Shumlib Release` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ **Dependencies** -All shumlib tickets +All shumlib PRs :ref:`Mule Release` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ **Dependencies** -All mule tickets, Shumlib release (if required), UM release (to actually +All mule PRs, Shumlib release (if required), UM release (to actually install) @@ -138,14 +138,14 @@ install) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ **Dependencies** -All UM Tickets, Test Release, Partner Testing, Jules Release +All UM PRs, Test Release, Partner Testing, Jules Release :ref:`LFRic Apps Release` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ **Dependencies** -All LFRic Tickets (Apps + Core), Jules Release +All LFRic PRs (Apps + Core), Jules Release Post Release Tasks diff --git a/source/Reviewers/howtocommit.rst b/source/Reviewers/howtocommit.rst index fe9c8c55..801144e8 100644 --- a/source/Reviewers/howtocommit.rst +++ b/source/Reviewers/howtocommit.rst @@ -3,7 +3,7 @@ Merge, Test & Commit ==================== -The process for committing a ticket follows this sequence with details for each +The process for committing a PR follows this sequence with details for each of these steps outlined below. .. image:: images/commit_process.png @@ -26,9 +26,9 @@ of these steps outlined below. .. important:: - **Linked Tickets?** + **Linked PRs?** - If this is a set of linked tickets then the commit process will need to be + If this is a set of linked PRs then the commit process will need to be followed for each repository in parallel. See :ref:`committinglinkedtickets` for more details of how this works. @@ -41,7 +41,7 @@ of these steps outlined below. .. important:: All changes made by the reviewer (e.g. for upgrade macros, KGOs or linked - tickets) will be committed to the developers branch in their fork. All + PRs) will be committed to the developers branch in their fork. All testing should be run on this branch too. Once this is all complete then GitHub can complete the merge of this branch to ``main``. @@ -51,7 +51,7 @@ of these steps outlined below. .. note:: - If this is a linked ticket, then do these steps for all pull requests + If this is a linked PR, then do these steps for all pull requests Ensure that the source branch is :ref:`up to date with main `. Only resolve conflicts that appear simple and you are comfortable with. If @@ -68,7 +68,7 @@ Then switch to the up to date branch, e.g. 2. Macros (if required) ----------------------- -**If** the ticket includes metadata changes, upgrade macro changes or a new +**If** the PR includes metadata changes, upgrade macro changes or a new rose-stem app then you will need to upgrade the test-suite. Update the versions.py file @@ -76,11 +76,12 @@ Update the versions.py file ``versions.py`` contains a sequence of upgrade macros. Each macro contains a ``BEFORE_TAG`` and an ``AFTER_TAG`` which should create a single chain, starting -at the last release and finishing with the ticket you are committing. The tags -have the format version_ticket, i.e. ``vnXX.Y_tZZZZ``. +at the last release and finishing with the PR you are committing. The tags +have the format version_ticket, i.e. ``vnXX.Y_tZZZZ``. The ticket value can be +either the Issue number of the PR number. When resolving conflicts in this file make sure that the new macro being added -by your ticket is added to the end of the file. Modify the ``BEFORE_TAG`` to +by your PR is added to the end of the file. Modify the ``BEFORE_TAG`` to match the ``AFTER_TAG`` of the previous macro in the chain. If this is the first macro since the release then the ``BEFORE_TAG`` will be the @@ -150,17 +151,12 @@ To update the test suite for an upgrade macro, please run: .. note:: - All LFRic Core tickets with macros are expected to be linked - with LFRic Apps, though they may not have required an LFRic - Apps development branch (although an Apps ticket should be - provided). This is fine - if there is no LFRic Apps branch - just checkout the LFRic Apps main. Then run the apply_macros - script as described above and this will share the upgrade - macro across both LFRic Apps and LFRic Core as needed. + All LFRic Core PRs with macros are expected to be linked with an + LFRic Apps change. .. dropdown:: New rose-stem app? - If the ticket introduces a new rose-stem app, but doesn't otherwise have a + If the PR introduces a new rose-stem app, but doesn't otherwise have a macro then that app will need to be updated to match the metadata at the Head Of Trunk. @@ -187,11 +183,11 @@ To update the test suite for an upgrade macro, please run: .. note:: - LFRic Apps tickets will require an LFRic Core source to use. You can do + LFRic Apps PRs will require an LFRic Core source to use. You can do this by checking out an appropriate working copy, and exporting the environment variable ``ROSE_META_PATH=/path/to/core``. - For UM tickets, if there are linked `jules-shared + For UM PRs, if there are linked `jules-shared `__ metadata changes then a suitable Jules source will need to be included in the `ROSE_META_PATH` as described above. @@ -214,21 +210,21 @@ To update the test suite for an upgrade macro, please run: -------------------- The amount of testing to be done at this stage depends on the complexity of the -ticket, and what has already been done. A minimum level is required for even -trivial tickets to check that the merge has not caused issues, or that there +PR, and what has already been done. A minimum level is required for even +trivial PRs to check that the merge has not caused issues, or that there are no clashes with what else has gone on trunk. .. note:: - Linked tickets will need to be tested together as discussed on - the :ref:`Committing Linked Tickets page`. + Linked PRs will need to be tested together as discussed on + the :ref:`Committing Linked PRs page`. .. tab-set:: .. tab-item:: UM Run any necessary testing; at the very least run a compile group, - generally run developer, and more complex tickets warrant running + generally run developer, and more complex PRs warrant running everything: .. code-block:: shell @@ -247,7 +243,7 @@ are no clashes with what else has gone on trunk. .. tab-item:: JULES The JULES test suite is quick to run, so it's usual to test ``all`` for - any ticket. If you have the appropriate environment setup then include + any PR. If you have the appropriate environment setup then include the ``fab`` group too. .. code-block:: shell @@ -265,7 +261,7 @@ are no clashes with what else has gone on trunk. cylc vip -z group=all -n ./rose-stem UKCA testing should also be carried out using the UM rose stem. See - :ref:`Linked Tickets page` for advice on how to set this + :ref:`Linked PRs page` for advice on how to set this up. @@ -375,7 +371,7 @@ KGO differently* 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 ticket. + #. 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 @@ -383,7 +379,7 @@ KGO differently* * 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 ticket number. + 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. @@ -411,7 +407,7 @@ KGO differently* .. dropdown:: Updating KGO manually (rarely needed!) * Create a new directory for the new KGO. The naming convention is - vnXX.X_tNNNN, where NNNN is the ticket number. The location of + vnXX.X_tNNNN, where NNNN is the PR number. The location of the KGO for the nightly is $UMDIR/standard_jobs. * Copy the new KGO from your rose-stem run into the directory vnXX.X_tNNNN created above. Note that you need to provide a @@ -597,13 +593,13 @@ branch and then push the changes back to the remote branch. .. important:: - Linked tickets will also need to update the relevant hashes for sub-repos - before pushing back to the fork. See :ref:`Committing Linked Tickets + Linked PRs will also need to update the relevant hashes for sub-repos + before pushing back to the fork. See :ref:`Committing Linked PRs ` for details. Once the remote branch has been updated, the pull request continuous integration will relaunch. Make sure this all passes and then you can commit -the ticket via the github interface, +the PR via the github interface, .. image:: images/gh_screenshots/merge_light.png :class: only-light border diff --git a/source/Reviewers/index.rst b/source/Reviewers/index.rst index a5a7d76e..e85ea405 100644 --- a/source/Reviewers/index.rst +++ b/source/Reviewers/index.rst @@ -3,13 +3,13 @@ Guides for Reviewers ==================== -Tickets will pass through a two-stage review. Linked tickets should work -through this process together. +PRs will pass through a two-stage review. Linked PRs should work through this +process together. The first, scitech, review is carried out by someone with a good understanding -of the area being developed. Once satisfied they then pass the ticket to a +of the area being developed. Once satisfied they then pass the PR to a code reviewer who takes a bigger picture and system view of the change. -Trivial tickets can bypass the scitech review. +Trivial PRs can bypass the scitech review. Once the reviews are complete the code reviewer is then responsible for merging the change to trunk and committing it. diff --git a/source/Reviewers/releases/jules_release.rst b/source/Reviewers/releases/jules_release.rst index 6ba25e93..0aface45 100644 --- a/source/Reviewers/releases/jules_release.rst +++ b/source/Reviewers/releases/jules_release.rst @@ -131,7 +131,7 @@ These are done with a PR in `this github repo `__ Often the release notes will have been prepared beforehand and have their own -ticket. In this case it makes more sense for you to review and commit that +PR. In this case it makes more sense for you to review and commit that branch. See below for the relevant steps and the how to commit page for instructions. @@ -142,7 +142,7 @@ detail any major commits. probably by copying from a previous release #. Go through the trunk commits since the last release and decide whether the change is worth noting -#. Use the ticket details to describe the change +#. Use the PR details to describe the change #. For some large commits, it is worth contacting the original author for a few sentences #. Add the new file to the contents, at the top - @@ -173,7 +173,7 @@ Update the wiki: * Create a new standard jobs page for the upcoming release cycle - compare the list of apps in the table to that in the rose-stem/apps directory. * Mark the wiki milestone for the release as completed (this should give the - option to move open tickets to a different milestone) + option to move open PRs to a different milestone) Notify the JULES community: diff --git a/source/Reviewers/releases/lfric_apps_release.rst b/source/Reviewers/releases/lfric_apps_release.rst index 2110e5fa..55e9363b 100644 --- a/source/Reviewers/releases/lfric_apps_release.rst +++ b/source/Reviewers/releases/lfric_apps_release.rst @@ -6,7 +6,7 @@ LFRic Apps Release LFRic Inputs KGO Install ------------------------ -* This can be done at any point once all tickets that change lfricinputs kgo +* This can be done at any point once all PRs that change lfricinputs kgo have been committed. * It's easiest to use the umtest nightly testing for this and will save having to run the suite twice. @@ -17,7 +17,7 @@ LFRic Inputs KGO Install * The script will ask for a working copy path - this can be any lfric apps working copy as it will not be modified. - * The version number and ticket number are not required, although an entry + * The version number and PR number are not required, although an entry is required. * The kgo install directory must be updated to vnX.Y @@ -43,7 +43,7 @@ LFRic Release * ``A.B`` - the previous version * ``X.Y`` - the new version - * ``TTTT`` - the apps release ticket number + * ``TTTT`` - the apps release PR number * ``/path/to/core`` - path to the lfric core clone * Check the output looks sensible. It should: diff --git a/source/Reviewers/releases/mule_release.rst b/source/Reviewers/releases/mule_release.rst index 46fee637..ef756694 100644 --- a/source/Reviewers/releases/mule_release.rst +++ b/source/Reviewers/releases/mule_release.rst @@ -15,7 +15,7 @@ Releasing Mule .. tip:: See `PR#2 `__ for an example mule - release ticket. + release PR. There's no hard rule on whether a mule release is required - it comes down to whether there have been any notable changes to mule since the last release. If diff --git a/source/Reviewers/releases/partner_testing.rst b/source/Reviewers/releases/partner_testing.rst index 1c953980..e90cfd9e 100644 --- a/source/Reviewers/releases/partner_testing.rst +++ b/source/Reviewers/releases/partner_testing.rst @@ -7,7 +7,7 @@ This is intended to allow partners to determine if there are any undetected problems with the head of the UM trunk (and its dependencies) before the release process gets started in earnest. -The partner testing period should be 72 hours (3 working days). All tickets +The partner testing period should be 72 hours (3 working days). All PRs affecting the main source code should be committed (to all repos) but limited scope changes, eg. to just the Met Office site, can still go on. diff --git a/source/Reviewers/releases/software_stack.rst b/source/Reviewers/releases/software_stack.rst index 6691aef0..ff13e6fd 100644 --- a/source/Reviewers/releases/software_stack.rst +++ b/source/Reviewers/releases/software_stack.rst @@ -7,7 +7,7 @@ Scientific Software Stack Update ``rose-stem/site/meto/variables.rc`` file and in ``admin/search_stash.sh`` * Decide whether this needs updating - using the latest production scitools environment is recommended. -* If you decide to update then complete with an associated ticket. +* If you decide to update then complete with an associated PR. .. note:: diff --git a/source/Reviewers/releases/um_main_release.rst b/source/Reviewers/releases/um_main_release.rst index c881987c..544e32d2 100644 --- a/source/Reviewers/releases/um_main_release.rst +++ b/source/Reviewers/releases/um_main_release.rst @@ -254,7 +254,7 @@ navigate to ``$UMDIR/SimSys_Scripts/kgo_updates`` directory and run * You will need to supply the username and suitename of the suite you ran above. This will need to include the ``runX`` directory. * The version number should be the new version. -* The ticket number won't be used but can be entered as the ticket associated +* The PR number won't be used but can be entered as the PR associated with the release. * When asked how the new kgo directory should be named overwrite the default with the name ``vnX.Y`` where this is the new version number. diff --git a/source/WorkingPractices/final_steps.rst b/source/WorkingPractices/final_steps.rst index 10fb65c4..bb059996 100644 --- a/source/WorkingPractices/final_steps.rst +++ b/source/WorkingPractices/final_steps.rst @@ -3,8 +3,8 @@ Final Steps Once your change is approved, the code reviewer will follow the :ref:`howtocommit` process on any branches in order to commit them to the trunk -of each project. Depending on any linked tickets and documentation updates, -there may be several commits to different trunks involved. +of each project. Depending on any linked PRs and documentation updates, there +may be several commits to different trunks involved. .. note:: @@ -24,13 +24,13 @@ usually based on the rose stem system. In addition, most projects run weekly tests, which involve some longer jobs not normally tested in the nightly tests. -Closing Tickets ---------------- +Closing Pull Requests +--------------------- -If the test suite runs overnight without issues, the CodeSys reviewer will -close the ticket(s) as 'fixed' and reassign them back to the developer. This -is usually the end of the process and the code changes will form part of the -next release. +GitHub will automatically close PRs upon merge. The code reviewer will check the +output of nightly testing, and if this shows errors may begin a discussion on +the pull request. Some testing is only run weekly, so some issues may take +longer to show up. When the Trunk is Broken @@ -41,26 +41,13 @@ immediately solved, the trunk(s) of affected projects will be closed to new changes. The relevant teams will investigate and aim to resolve the issue and reopen the trunk(s) as soon as possible. Two possible scenarios may occur: - #. For **simple or obvious fixes**, a second commit is the preferred - solution, allowing the change to be fixed, while remaining on the trunk. + #. For **simple or obvious fixes**, a trivial PR to fix the trunk is + preferred. #. If the reason for the failure is complex or less obvious, the team will revert the offending change off the trunk(s). -In the first case, if the test suite comes back clean, the ticket will be -closed, as above. In the second case, the ticket will be returned to the -original developer, allowing them to fix the issue for a later commit, either -during the current release if time permits, or alternatively during a later -release cycle. - -Reopening Tickets ------------------ - -Very rarely, an issue will be discovered with a ticket some time after it has -been committed to the trunk. The most common case is when the nightly tests -pass, but the weekly tests fail. In this case, either the initial ticket will -be either be reopened and a fix found, or a further ticket will be created to -investigate the issue. +In both cases any further PRs should link back to the original. Changes to the Working Practices diff --git a/source/WorkingPractices/gh_dev_init.rst b/source/WorkingPractices/gh_dev_init.rst index ed92f1d0..a55054b0 100644 --- a/source/WorkingPractices/gh_dev_init.rst +++ b/source/WorkingPractices/gh_dev_init.rst @@ -140,7 +140,7 @@ Create a Branch Branches for developing Simulation Systems repositories should generally be branched from ``stable`` where this exists (some smaller repositories only contain a ``main`` branch). Creating a branch from ``main`` may be acceptable -if the development is continuing on from a ticket already committed at that +if the development is continuing on from a PR already committed at that release. To create a branch and switch to it from the command line, the syntax is, diff --git a/source/WorkingPractices/working_practices.rst b/source/WorkingPractices/working_practices.rst index ce4cca48..9a945c2c 100644 --- a/source/WorkingPractices/working_practices.rst +++ b/source/WorkingPractices/working_practices.rst @@ -126,10 +126,10 @@ The development cycle can be seen below. create_pr -> test_change; test_change [label="Test Changes\nRose Stem locally + CI in PR"] - test_change -> scitech -> code_review -> commit_ticket; + test_change -> scitech -> code_review -> commit_pr; scitech [label="SciTech Review", style=filled, color="#f8c6b4"] code_review[label="Code Review", style=filled, color="#f8c6b4"] - commit_ticket[label="Commit branch to main", style=filled, color="#f8c6b4"] + commit_pr[label="Commit branch to main", style=filled, color="#f8c6b4"] merge_main -> test_change [style="dashed", label="As Required"]; merge_main [label="Merge in upstream/main", style=filled, color="#8eb6e8"] @@ -216,7 +216,7 @@ Consider the timing of your work: * Allow contingency time when agreeing broader project deadlines. Trunk integrity will not be compromised to meet your deadlines. -Consider bringing planning together using an overarching ticket. It can be very +Consider bringing planning together using an overarching issue. It can be very helpful for documenting and monitoring progress of your work. .. tip:: From c36ec7464096b2e37b97918a956428339eaf7a8d Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:29:05 +0100 Subject: [PATCH 02/20] Update source/Development/inputs.rst Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/inputs.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/Development/inputs.rst b/source/Development/inputs.rst index 5aa12c31..6ee47ca4 100644 --- a/source/Development/inputs.rst +++ b/source/Development/inputs.rst @@ -102,9 +102,9 @@ whether an upgrade macro is necessary. cd rose-meta rose edit -C ../rose-stem/app/APP-NAME --no-warn version - If you have a linked LFRic Core or Jules PR with metadata changes, you - can load their metadata by adding ``-M /path/to/working_copy/rose-meta`` to - the ``rose-edit`` command. + If you have a linked LFRic Core or Jules development with metadata changes, + you can load their metadata by adding + ``-M /path/to/working_copy/rose-meta`` to the ``rose-edit`` command. Adding a new LFRic Metadata Section From ada5294c8b3897181eff47f260bdf43f74a4f20b Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:45:58 +0100 Subject: [PATCH 03/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/TestSuites/ukca.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/Development/TestSuites/ukca.rst b/source/Development/TestSuites/ukca.rst index 88bacf7a..8b05bc32 100644 --- a/source/Development/TestSuites/ukca.rst +++ b/source/Development/TestSuites/ukca.rst @@ -2,8 +2,8 @@ Testing UKCA ============ Changes in UKCA that touch `src/science` or `src/control/core` must be tested -with both the UM and LFRic by following the :ref:`linked PRs guidance -`. +with both the UM and LFRic by following the :ref:`linked pull requests +guidance `. For further guidance on testing and working with UKCA, including standard suites and box models see the `UKCA trac wiki From e5c6a45a5cb20ea095fdb4999af5fdf473c504fd Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:47:03 +0100 Subject: [PATCH 04/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/kgo.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index be387035..276cd731 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -13,8 +13,8 @@ may also be required to simply add a new job to the test suite or to port the rose stem suite to new HPC architecture. **LFRic** KGO checksums are stored in the repository. As such with LFRic -PRs the expectation is that you, as the developer, will include updated -KGO files as part of your branch. +pull requests the expectation is that you, as the developer, will include +updated KGO files as part of your branch. **UM and JULES** KGO output files are stored outside of the repository. Access to this area is restricted to members of the Simulation Systems and Deployment From c714123a570fabf15aae4142b4e906ee2fa67ad8 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:47:11 +0100 Subject: [PATCH 05/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/kgo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index 276cd731..9b7cff4c 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -25,7 +25,7 @@ the commit process. KGO Update Process ------------------ -Getting the process right for KGO changing PRs significantly helps get such +Getting the process right for KGO changing pull requests significantly helps get such changes onto the trunk. When preparing your change for review: 1. Run the ``all`` rose-stem group in order to make sure that all From b523c343e43a913c414444b7db4deea08894b0df Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:47:22 +0100 Subject: [PATCH 06/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/kgo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index 9b7cff4c..144ea94c 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -32,7 +32,7 @@ changes onto the trunk. When preparing your change for review: changes to answers have been found. * Include the :ref:`trac.log ` output from this testing in your - PR summary. + pull request summary. 2. Add the ``kgo`` label to your PR. From 2823f1c39e52e196dc4b379608bbee2bb841dd7e Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:47:30 +0100 Subject: [PATCH 07/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/kgo.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index 144ea94c..4d8c8f70 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -34,7 +34,7 @@ changes onto the trunk. When preparing your change for review: * Include the :ref:`trac.log ` output from this testing in your pull request summary. -2. Add the ``kgo`` label to your PR. +2. Add the ``kgo`` label to your pull request. .. tab-set:: From 2dc1649e74b9f1c747fe0fa6972ff8eff00e3f11 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:47:39 +0100 Subject: [PATCH 08/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/metadata_guidance.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/metadata_guidance.rst b/source/Development/metadata_guidance.rst index 4a791ceb..7682d309 100644 --- a/source/Development/metadata_guidance.rst +++ b/source/Development/metadata_guidance.rst @@ -63,7 +63,7 @@ more details including what should be in `jules-shared `__ and in `jules-standalone, jules-lfric or um-atmos `__. -When developing shared JULES metadata, you will need :ref:`linked PRs +When developing shared JULES metadata, you will need :ref:`linked pull requests `. The metadata migration is currently dictated by LFRic porting of science, although the ultimate aim is to have a single source of truth. From 74c97b2babb49fe19a69bd2674850732bf7a4be6 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:01 +0100 Subject: [PATCH 09/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/planning_your_change.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/planning_your_change.rst b/source/Development/planning_your_change.rst index f1982826..35c7e58b 100644 --- a/source/Development/planning_your_change.rst +++ b/source/Development/planning_your_change.rst @@ -77,7 +77,7 @@ approvals to take place. **Is the code you need to alter on a single repository or is it spread over multiple repositories?** If it's over multiple repositories you need to use -linked PRs. See :ref:`multirepo` for further details. +linked pull requests. See :ref:`multirepo` for further details. **Does similar code functionality already exist in the model?** It's a good idea **not** to re-invent the wheel or have code duplication! Speaking to code From c05400adaf7cce1402f81ea00b3dabf66a0d0d1e Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:10 +0100 Subject: [PATCH 10/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/temp_logicals.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/temp_logicals.rst b/source/Development/temp_logicals.rst index 684a4fbb..0aee52fe 100644 --- a/source/Development/temp_logicals.rst +++ b/source/Development/temp_logicals.rst @@ -156,7 +156,7 @@ temporary logical should be removed from the code base. .. important:: - Prior to a PR containing a temporary logical being committed to the + Prior to a pull request containing a temporary logical being committed to the trunk, the developers is expected to open a new Issue which removes the logical after a fixed period. This acts as an memory aid that the logical needs to be removed in due course. From a3f9530a69df173a977f95b100b77698fadc9e59 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:23 +0100 Subject: [PATCH 11/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/dos_donts.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FurtherDetails/dos_donts.rst b/source/FurtherDetails/dos_donts.rst index 337bcc26..e70967fe 100644 --- a/source/FurtherDetails/dos_donts.rst +++ b/source/FurtherDetails/dos_donts.rst @@ -15,7 +15,7 @@ sometimes years later. * Each pull request should contain a single coherent change. * Consider using an overarching issue to link everything together -**Document your work** using Issues and PRs pages, :ref:`formal documentation +**Document your work** using Issues and pull requests, :ref:`formal documentation ` and code comments. These help others and your future-self understand your work. From 8497f516530fd79f25d72bb277dd99731d5b8cdc Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:32 +0100 Subject: [PATCH 12/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/dos_donts.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/FurtherDetails/dos_donts.rst b/source/FurtherDetails/dos_donts.rst index e70967fe..27f74d42 100644 --- a/source/FurtherDetails/dos_donts.rst +++ b/source/FurtherDetails/dos_donts.rst @@ -19,8 +19,9 @@ sometimes years later. ` and code comments. These help others and your future-self understand your work. -**Meaningful names** for issues, PRs, branches and variables. These help others -and your future-self understand your work. "My_Branch", "Fix" are not helpful. +**Meaningful names** for issues, pull requests, branches and variables. These +help others and your future-self understand your work. "My_Branch", "Fix" are +not helpful. **Be considerate** of other users/developers. Their skill-sets and working days may be very different to yours. All changes are visible to all users From f2e6704492772b70060ce95e89987797ce016ab3 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:40 +0100 Subject: [PATCH 13/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/dos_donts.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FurtherDetails/dos_donts.rst b/source/FurtherDetails/dos_donts.rst index 27f74d42..cab94bc9 100644 --- a/source/FurtherDetails/dos_donts.rst +++ b/source/FurtherDetails/dos_donts.rst @@ -27,7 +27,7 @@ not helpful. may be very different to yours. All changes are visible to all users worldwide. -**Link to issues/PRs in other repositories**, eg ``MetOffice/jules#1``, +**Link to issues/pull requests in other repositories**, eg ``MetOffice/jules#1``, ``MetOffice/ukca:#72`` Please Do Not From 1bbc1f862a45310d4905a40558ea5d6a4343fc09 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:48:48 +0100 Subject: [PATCH 14/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/glossary.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FurtherDetails/glossary.rst b/source/FurtherDetails/glossary.rst index c531eda6..ea8df077 100644 --- a/source/FurtherDetails/glossary.rst +++ b/source/FurtherDetails/glossary.rst @@ -10,7 +10,7 @@ Closed Release: this way, and the rest free for changes. Code Review Deadline: - The date by which all PRs aiming to be included in a release have been + The date by which all pull requests aiming to be included in a release have been moved into code review. CodeSys Review: From 66d7358327b475fec0e52f364773ef0225d27d63 Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:49:02 +0100 Subject: [PATCH 15/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/glossary.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FurtherDetails/glossary.rst b/source/FurtherDetails/glossary.rst index ea8df077..4d444792 100644 --- a/source/FurtherDetails/glossary.rst +++ b/source/FurtherDetails/glossary.rst @@ -36,7 +36,7 @@ Head of Trunk: when the work being done *has* to be built on top of changes already made since the last revision. -Linked PR: +Linked Pull Request (Linked PR): Work that spans two or more repositories, requiring PRs that should be treated together and committed as a group. From a4d24a74e2ac5b77f2242067af4514e88f14169f Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:49:14 +0100 Subject: [PATCH 16/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/glossary.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/FurtherDetails/glossary.rst b/source/FurtherDetails/glossary.rst index 4d444792..8705e221 100644 --- a/source/FurtherDetails/glossary.rst +++ b/source/FurtherDetails/glossary.rst @@ -37,8 +37,8 @@ Head of Trunk: since the last revision. Linked Pull Request (Linked PR): - Work that spans two or more repositories, requiring PRs that should be - treated together and committed as a group. + Work that spans two or more repositories, requiring pull requests that + should be treated together and committed as a group. Overarching Issue: Where a piece of work has been split into multiple sections an overarching From 8a2481f6d95ae8f1e2ee6e9d14c6eff453a4425f Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:49:32 +0100 Subject: [PATCH 17/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/who.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/FurtherDetails/who.rst b/source/FurtherDetails/who.rst index c60a41ac..fa6e1a5d 100644 --- a/source/FurtherDetails/who.rst +++ b/source/FurtherDetails/who.rst @@ -79,10 +79,10 @@ of model evolution. Sci/Tech Reviewer ----------------- -A Sci/Tech reviewer is assigned for every PR and comprises the first stage -of review that considers the change as a whole. Further details are found in -the :ref:`Scitech Reviewers Guide`. In some cases, the reviewer -can delegate parts of the work to another person. +A Sci/Tech reviewer is assigned for every pull request and comprises the first +stage of review that considers the change as a whole. Further details are found +in the :ref:`Scitech Reviewers Guide`. In some cases, the +reviewer can delegate parts of the work to another person. Reviews should be turned around on a reasonable timescale and follow the SciTech review guidance. From dcb502e9164b74f7485811c20a9a74a025899ceb Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:49:47 +0100 Subject: [PATCH 18/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/FurtherDetails/who.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/FurtherDetails/who.rst b/source/FurtherDetails/who.rst index fa6e1a5d..d2e7a169 100644 --- a/source/FurtherDetails/who.rst +++ b/source/FurtherDetails/who.rst @@ -96,8 +96,9 @@ most relevant code owner. Code Reviewer ------------- -The Code Reviewer performs the 2nd stage of review for every PR. Further -details are described in the :ref:`Code Reviewers Guide`. +The Code Reviewer performs the 2nd stage of review for every pull +request. Further details are described in the :ref:`Code Reviewers +Guide`. Reviews should be turned around on a reasonable timescale and follow the Code Review guidance. From fde0b63eb75341242b4068edaba246a2bce4a9dd Mon Sep 17 00:00:00 2001 From: James Bruten <109733895+james-bruten-mo@users.noreply.github.com> Date: Fri, 24 Oct 2025 09:54:46 +0100 Subject: [PATCH 19/20] Apply suggestion from @jennyhickson Co-authored-by: Jenny Hickson <61183013+jennyhickson@users.noreply.github.com> --- source/Development/planning_your_change.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/Development/planning_your_change.rst b/source/Development/planning_your_change.rst index 35c7e58b..7880bb3c 100644 --- a/source/Development/planning_your_change.rst +++ b/source/Development/planning_your_change.rst @@ -98,7 +98,7 @@ upgrade macro to include the switch into the upgraded configuration. vital to make sure your code changes work when switched **on** and give the same answer when the code is run over different processor configurations. Producing a quick plot or plots to show the impact of your code and including -them on your PR can aid your SciTech reviewer in showing that your code +them on your pull request can aid your SciTech reviewer in showing that your code works properly. **Does the change need any new diagnostics to make sense of the code?** Many From 8d00ca51b96c20d200e73f79f4f80a41d2f04850 Mon Sep 17 00:00:00 2001 From: James Bruten Date: Fri, 24 Oct 2025 09:59:03 +0100 Subject: [PATCH 20/20] reviewer suggestions --- source/Development/TestSuites/multi-repo_testing.rst | 8 ++++---- source/Development/TestSuites/ukca.rst | 2 +- source/Development/inputs.rst | 6 +++--- source/Development/kgo.rst | 2 +- source/Development/planning_your_change.rst | 2 +- source/FurtherDetails/dos_donts.rst | 2 +- source/FurtherDetails/glossary.rst | 11 ++++++----- source/FurtherDetails/who.rst | 10 +++++----- 8 files changed, 22 insertions(+), 21 deletions(-) diff --git a/source/Development/TestSuites/multi-repo_testing.rst b/source/Development/TestSuites/multi-repo_testing.rst index 209a847d..d2181953 100644 --- a/source/Development/TestSuites/multi-repo_testing.rst +++ b/source/Development/TestSuites/multi-repo_testing.rst @@ -4,10 +4,10 @@ Multi-Repository Testing ======================== Multi-repository changes are expected to pass the regression tests for all the -repositories involved. To carry out the tests involved in a linked PR it can be -helpful to refer to the :ref:`repository figure `; testing both child -and parent repositories as needed. Further details of how testing in each -repository is handled can be found on the :ref:`Testing page`. +repositories involved. To carry out the tests involved in a linked pull request +it can be helpful to refer to the :ref:`repository figure `; testing +both child and parent repositories as needed. Further details of how testing in +each repository is handled can be found on the :ref:`Testing page`. All Simulation Systems repositories containing a test suite will also contain a ``dependencies.yaml`` file in the top directory of the repository. This file diff --git a/source/Development/TestSuites/ukca.rst b/source/Development/TestSuites/ukca.rst index 8b05bc32..5c8928c8 100644 --- a/source/Development/TestSuites/ukca.rst +++ b/source/Development/TestSuites/ukca.rst @@ -2,7 +2,7 @@ Testing UKCA ============ Changes in UKCA that touch `src/science` or `src/control/core` must be tested -with both the UM and LFRic by following the :ref:`linked pull requests +with both the UM and LFRic by following the :ref:`linked pull requests guidance `. For further guidance on testing and working with UKCA, including standard diff --git a/source/Development/inputs.rst b/source/Development/inputs.rst index 6ee47ca4..55064782 100644 --- a/source/Development/inputs.rst +++ b/source/Development/inputs.rst @@ -102,8 +102,8 @@ whether an upgrade macro is necessary. cd rose-meta rose edit -C ../rose-stem/app/APP-NAME --no-warn version - If you have a linked LFRic Core or Jules development with metadata changes, - you can load their metadata by adding + If you have a linked LFRic Core or Jules development with metadata changes, + you can load their metadata by adding ``-M /path/to/working_copy/rose-meta`` to the ``rose-edit`` command. @@ -123,7 +123,7 @@ Adding a new metadata section requires: (see existing ones for examples) The ``vnX.Y`` and ``HEAD`` metadata should be identical for this initial -PR, other than any import statements, which should point at vnX.Y or HEAD +change, other than any import statements, which should point at vnX.Y or HEAD respectively. Other ``vnX.Y`` and ``versionAB_CD.py`` files shouldn't be modified or added (these are a snapshot of the metadata at a release). diff --git a/source/Development/kgo.rst b/source/Development/kgo.rst index 4d8c8f70..0197ad2b 100644 --- a/source/Development/kgo.rst +++ b/source/Development/kgo.rst @@ -13,7 +13,7 @@ may also be required to simply add a new job to the test suite or to port the rose stem suite to new HPC architecture. **LFRic** KGO checksums are stored in the repository. As such with LFRic -pull requests the expectation is that you, as the developer, will include +pull requests the expectation is that you, as the developer, will include updated KGO files as part of your branch. **UM and JULES** KGO output files are stored outside of the repository. Access diff --git a/source/Development/planning_your_change.rst b/source/Development/planning_your_change.rst index 7880bb3c..e20870e0 100644 --- a/source/Development/planning_your_change.rst +++ b/source/Development/planning_your_change.rst @@ -64,7 +64,7 @@ aware of these. **Who will SciTech review the change?** This is a useful consideration as not everyone who uses the repository has the knowledge or experience to review -every PR that is being developed. Get in touch with your SciTech reviewer +every change that is being developed. Get in touch with your SciTech reviewer early in the process as they will have valuable insights that can help to shape your change. diff --git a/source/FurtherDetails/dos_donts.rst b/source/FurtherDetails/dos_donts.rst index cab94bc9..c36528a4 100644 --- a/source/FurtherDetails/dos_donts.rst +++ b/source/FurtherDetails/dos_donts.rst @@ -19,7 +19,7 @@ sometimes years later. ` and code comments. These help others and your future-self understand your work. -**Meaningful names** for issues, pull requests, branches and variables. These +**Meaningful names** for issues, pull requests, branches and variables. These help others and your future-self understand your work. "My_Branch", "Fix" are not helpful. diff --git a/source/FurtherDetails/glossary.rst b/source/FurtherDetails/glossary.rst index 8705e221..409f757d 100644 --- a/source/FurtherDetails/glossary.rst +++ b/source/FurtherDetails/glossary.rst @@ -14,9 +14,9 @@ Code Review Deadline: moved into code review. CodeSys Review: - A technical review of the changes involved in the PR, including checks - that code standards have been upheld and that the working practices have - been followed. These reviews are generally completed by a member of the + A technical review of the changes involved in the pull request, including + checks that code standards have been upheld and that the working practices + have been followed. These reviews are generally completed by a member of the Simulation IT Team. Once a review has been approved the Code Systems Reviewer is then responsible for committing the change to the trunk. @@ -37,13 +37,14 @@ Head of Trunk: since the last revision. Linked Pull Request (Linked PR): - Work that spans two or more repositories, requiring pull requests that + Work that spans two or more repositories, requiring pull requests that should be treated together and committed as a group. Overarching Issue: Where a piece of work has been split into multiple sections an overarching issue can be used to track this work. It should be closed when the whole arc - has been completed. + has been completed. GitHub also allows subissues which can be created from + the overarching issue. Regression: A set of tests that prove that a set of code changes have not degraded the diff --git a/source/FurtherDetails/who.rst b/source/FurtherDetails/who.rst index d2e7a169..d2b49d1c 100644 --- a/source/FurtherDetails/who.rst +++ b/source/FurtherDetails/who.rst @@ -79,9 +79,9 @@ of model evolution. Sci/Tech Reviewer ----------------- -A Sci/Tech reviewer is assigned for every pull request and comprises the first -stage of review that considers the change as a whole. Further details are found -in the :ref:`Scitech Reviewers Guide`. In some cases, the +A Sci/Tech reviewer is assigned for every pull request and comprises the first +stage of review that considers the change as a whole. Further details are found +in the :ref:`Scitech Reviewers Guide`. In some cases, the reviewer can delegate parts of the work to another person. Reviews should be turned around on a reasonable timescale and follow the @@ -96,8 +96,8 @@ most relevant code owner. Code Reviewer ------------- -The Code Reviewer performs the 2nd stage of review for every pull -request. Further details are described in the :ref:`Code Reviewers +The Code Reviewer performs the 2nd stage of review for every pull +request. Further details are described in the :ref:`Code Reviewers Guide`. Reviews should be turned around on a reasonable timescale and follow the Code