diff --git a/source/WorkingPractices/forking.rst b/source/WorkingPractices/forking.rst index 11233c35..7e5b17ef 100644 --- a/source/WorkingPractices/forking.rst +++ b/source/WorkingPractices/forking.rst @@ -61,6 +61,25 @@ the owner. Add ``--clone`` to immediately clone the forked repo + +Disable Github Actions in your fork +----------------------------------- + +It is a good idea to disable github actions on your fork otherwise you will +regularly get failures when updating your main branch, due to the failure to +deploy documentation. Doing this will not prevent the CI running when you open a +pull request. + +To disable, in your fork on github, navigate to Settings and then +Actions/General. There select the option to ``Disable actions`` and save. + +.. image:: images/gh_screenshots/disable_actions_light.png + :class: only-light border + +.. image:: images/gh_screenshots/disable_actions_dark.png + :class: only-dark border + + Optional - Adding the MetOffice Mirror Bot ------------------------------------------ diff --git a/source/WorkingPractices/gh_dev_init.rst b/source/WorkingPractices/gh_dev_init.rst index 7e1d2c70..50815abc 100644 --- a/source/WorkingPractices/gh_dev_init.rst +++ b/source/WorkingPractices/gh_dev_init.rst @@ -7,6 +7,8 @@ This section will guide you through the development process assuming you are already authorised with github (:ref:`gh_authorisation`) and have already created a fork (:ref:`forking`). +.. _create_issue: + Create an Issue --------------- @@ -106,7 +108,8 @@ general development, you should now get a clone of your fork. .. image:: images/gh_screenshots/clone_button_dark.png :class: only-dark border - selecting the url as desired. + selecting the url as desired. Note, that this will likely be a fork + rather than the upstream MetOffice repository shown above. .. _create_branch: diff --git a/source/WorkingPractices/images/gh_screenshots/disable_actions_dark.png b/source/WorkingPractices/images/gh_screenshots/disable_actions_dark.png new file mode 100644 index 00000000..2be45227 Binary files /dev/null and b/source/WorkingPractices/images/gh_screenshots/disable_actions_dark.png differ diff --git a/source/WorkingPractices/images/gh_screenshots/disable_actions_light.png b/source/WorkingPractices/images/gh_screenshots/disable_actions_light.png new file mode 100644 index 00000000..94d47b3d Binary files /dev/null and b/source/WorkingPractices/images/gh_screenshots/disable_actions_light.png differ diff --git a/source/WorkingPractices/images/git-dev-strategy.svg b/source/WorkingPractices/images/git-dev-strategy.svg deleted file mode 100644 index 4051eacf..00000000 --- a/source/WorkingPractices/images/git-dev-strategy.svg +++ /dev/null @@ -1,99 +0,0 @@ -
Stable Upstream
MetOffice/{repo}
Stable Upstream...
Create Downstream Fork
from Upstream
Create Downstream Fo...
Initial Setup
Initial Setup
Create Issue# Upstream
Create Issue# Upstre...
Forked repo
Downstream
{user}/{repo}
Forked repo...
Open Draft PR#
Link Issue#
Downstream
Open Draft PR#...

Branch off Stable Downstream
Branch off Stable Do...
Development Work
Downstream
Development Work...
Additional Development
Downstream
Additional Developme...
Science Review
Upstream
Science Review...
Test Suite + CI
Upstream and
Downstream
Test Suite + CI...
Fail
Fail
Ready for
Review
Ready for...
Changes
Required
Changes...
Code Review
Upstream
Code Review...
Changes
Required
Changes...
Sync Fork
Merge in main
Downstream
Sync Fork...
As
Required
As...
Merge to main
Upstream
Merge to main...
Text is not SVG - cannot display
\ No newline at end of file diff --git a/source/WorkingPractices/pull_requests.rst b/source/WorkingPractices/pull_requests.rst index c461ae05..4b0e2426 100644 --- a/source/WorkingPractices/pull_requests.rst +++ b/source/WorkingPractices/pull_requests.rst @@ -121,7 +121,7 @@ Initially you can choose to do this in draft mode, to allow you time to do any final fixes based on continuous integration. **If you use draft mode mark the pull request as ``ready for review`` once you are satisfied.** - +.. _CI: Continuous Integration ---------------------- @@ -154,6 +154,8 @@ rejected with failing tests. .. image:: images/gh_screenshots/ci_dark.png :class: only-dark border +.. _merge_main: + Updating a branch ----------------- @@ -178,6 +180,13 @@ There is a space in the pull request template to list the GitHub user ID of the scitech and code reviewers. Once these are filled in a GitHub Action will add this user as an ``assignee`` to the pull request. +.. tip:: + + Github allows reviewers to directly make suggestions to the code. This is + very useful for easily suggesting changes. However, the developer should + always check carefully that the change is sensible and doesn't contain any + errors or bugs. + SciTech Review ^^^^^^^^^^^^^^ diff --git a/source/WorkingPractices/working_practices.rst b/source/WorkingPractices/working_practices.rst index 0971126f..ce4cca48 100644 --- a/source/WorkingPractices/working_practices.rst +++ b/source/WorkingPractices/working_practices.rst @@ -43,46 +43,58 @@ release to facilitate an intense or disruptive development. :class: only-dark The release cycle is overseen by the Simulation Systems and Deployment Team -with the oversight and support of the Simulation Systems Governance Group, who +with the oversight and support of the LFRic Apps Governance Group, who impartially consider the needs of all developers and users. +Git/Github Definitions +---------------------- -Development Process -------------------- +Some commonly used git/github definitions. Further definitions can be found in +the `GitHub Glossary +`__. -The process of developing a change for each repository is described through -these Working Practices. A flowchart of this process for the UM is included -below, but the process is very comparable to that of the other repositories -too. +Clone + A local copy of either the upstream or the forked repository. -.. note:: +Continuous Integration + Testing that is run on github based on the changes in a pull request. For + simulation-sytems repositories, this is run in addition to local rose-stem + testing. It is commonly referred to as CI. - In the following Working Practices, we will endeavour to provide options - for using the Github Web Interface and the ``gh`` cli where possible. - Further information will be available in the github documentation. - - To get started with the ``gh`` cli, see the `gh quickstart guide - `__ +Fork + A copy of the upstream repository, owned by the developer. This is where + development branches are created and worked on. (May also be referred to + as the downstream repository). -.. admonition:: Definitions +Issue + An issue is a location to record tasks, problems, questions etc. in a + repository. They contain their own discussion thread and can be used as a way + of tracking work and recording details that might not be appropriate in a pull + request. - ``Clone``: a local copy of either the upstream or the forked repository. +Origin + The default name for the remote source of a cloned repository. - ``Fork`` or ``Downstream``: a copy of the upstream repository, owned by the - developer. This is where development branches are created and worked on. +Pull Request + These represent proposed changes to a repository, submitted by a developer. + They will undergo a review process before being merged onto the repository if + accepted. They are often referred to as PRs. - ``Origin``: the default name for the remote source of a cloned repository. +Remote + The version of either the upstream or the forked repository that is hosted + by Github. - ``Remote``: the version of either the upstream or the forked repository - that is hosted by Github. +Upstream + The primary or parent repository, owned by the MetOffice github + organisation. Only code reviewers are able to directly interact with this + repository, rather than being required to use forks. - ``Upstream``: the primary or parent repository, owned by the MetOffice - github organisation. Only code reviewers are able to directly interact - with this repository, rather than being required to use forks. +Development Process +------------------- - Further definitions can be found in the `GitHub Glossary - `__. +The process of developing a change for each repository is described through +these Working Practices. A flowchart of this process is included below. Simulation Systems github repositories are setup with at least 2 protected branches, ``main`` and ``stable`` (with the potential for additional version @@ -102,39 +114,99 @@ to maintain their own fork. See :ref:`forking` for advice on forking. The development cycle can be seen below. -.. image:: images/git-dev-strategy.svg - :class: dark-light +.. graphviz:: + + digraph { + create_issue -> create_branch -> development -> create_pr; + create_issue [label="Create an Issue", style=filled, color="#f8c6b4"] + create_branch [label="Create a Development Branch", style=filled, color="#8eb6e8"] + development [label="Develop Changes on Branch", style=filled, color="#8eb6e8"] + create_pr [label="Create a PR for the change", style=filled, color="#f8c6b4"] + + create_pr -> test_change; + test_change [label="Test Changes\nRose Stem locally + CI in PR"] + + test_change -> scitech -> code_review -> commit_ticket; + 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"] + + merge_main -> test_change [style="dashed", label="As Required"]; + merge_main [label="Merge in upstream/main", style=filled, color="#8eb6e8"] + + test_change -> code_review [dir=back, style="dashed", label="Changes\nRequired", color="#8eb6e8"]; -For detailed explanation of these steps, see pages on :ref:`gh_dev_init` and -:ref:`pull_requests`. + subgraph cluster0 { + upstream -> fork [style=invis]; + upstream [label="Takes place in the\nupstream repository", style=filled, color="#f8c6b4", shape=box] + fork [label="Takes place in the\nforked repository", style=filled, color="#8eb6e8", shape=box] + } + } + +#. :ref:`Create an Issue ` in the upstream repository to document + your changes. +#. :ref:`Create a branch ` in your fork of the repo. The branch + should usually be created from the stable branch. +#. Develop your change on your new branch. See the :ref:`development guide + ` for advice on how to do this, including running testing. +#. When ready for review :ref:`create a pull request ` in the + upstream repository. + + * Ensure that your change is passing the :ref:`Continuous Integration ` + and you have included proof of local testing. + +#. Pass the pull request for review. Usually this will involve first a + :ref:`Sci Tech review ` and then a :ref:`Code review + `. If any changes are required ensure the testing still passes. + + * Once in code review, you will likely need to update your branch to the + :ref:`head of main `. This may involve solving merge conflicts. + +#. Once the pull request has been approved, the code reviewer will merge it to + `main`. + +For detailed explanation of these steps, see the subsequent pages, particularly +on :ref:`gh_dev_init`, :ref:`maintaining_forks` and :ref:`pull_requests`. Before You Start ---------------- -All developments should be planned using a risk-based approach. Before -starting, consider the complexity and impact of what you want to do. This will -act as a guide for the level of planning and consultation required. There is -no definitive process for this and developers should use their experience and -judgement. +.. note:: + + In the following Working Practices, we will endeavour to provide options + for using the Github Web Interface and the ``gh`` cli where possible. + Further information will be available in the github documentation. + + To get started with the ``gh`` cli, see the `gh quickstart guide + `__ + +All developments should be :ref:`planned ` using a risk-based +approach. Before starting, consider the complexity and impact of what you want +to do. This will act as a guide for the level of planning and consultation +required. There is no definitive process for this and developers should use +their experience and judgement. As you begin, there are various people you might consider consulting: * Relevant :ref:`Code and Configuration Owners ` * Simulation Systems and Deployment Team * Core Capability Development Team +* Tools and Collaborative Development Team * Less experienced developers may benefit from a 'buddy' -For larger changes, consider splitting the work over multiple tickets: - -* Tickets laying foundations for later are OK -* Tickets should make sense on their own, with a clear scope, to allow for - separate testing, review and commit -* Tickets should not be too small or too large -* Beware of the 'also trap'- the 'also' bits can swamp the main aim of your - change! -* An overarching ticket that sets out the overall picture and tracks the - progress of the work is recommended and all sub-tickets should link back to - it +For larger changes, split the work over multiple Issues and Pull +Requests: + +* An overarching issue that sets out the overall picture and tracks the + progress of the work is encouraged. All sub-issues and pull requests should + link back to it. +* Each pull request should contain a single coherent change. + + * Pull requests may build on each other, however each should produce a valid + branch, as detemined by the test suite. + * The new feature being developed doesn't need to fully work after each pull + request, but the plan for developing it should be documented on the + overarching issue. Consider the timing of your work: diff --git a/source/conf.py b/source/conf.py index b38c840a..1e4f171b 100644 --- a/source/conf.py +++ b/source/conf.py @@ -23,9 +23,6 @@ # Add any Sphinx extension module names here, as strings. They can be # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. -extensions = [ - 'sphinx_sitemap' -] language = "en" @@ -34,6 +31,7 @@ 'sphinx_design', 'sphinx_copybutton', 'sphinxcontrib.rsvgconverter', + 'sphinx.ext.graphviz' ] # Add any paths that contain templates here, relative to this directory.