From b5f49de1f237b20cb00f52b6954402fa4c82cda5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Tue, 6 Feb 2024 21:07:24 +0100 Subject: [PATCH] Add section on keeping CI green (#1215) Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Jelle Zijlstra Co-authored-by: Ezio Melotti --- getting-started/git-boot-camp.rst | 8 ++-- getting-started/pull-request-lifecycle.rst | 50 ++++++++++++++++++++-- triage/triaging.rst | 1 + 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/getting-started/git-boot-camp.rst b/getting-started/git-boot-camp.rst index 2bee5c29fe..2fb5e2f720 100644 --- a/getting-started/git-boot-camp.rst +++ b/getting-started/git-boot-camp.rst @@ -476,9 +476,11 @@ You can read more about what to look for before accepting a change :ref:`here `. All pull requests have required checks that need to pass before a change -can be merged. At any point, a core developer can schedule an automatic merge -of the change by -clicking the gray ``Enable auto-merge (squash)`` button. You will find +can be merged. See :ref:`"Keeping CI green" ` for some +simple things you can do to help the checks turn green. + +At any point, a core developer can schedule an automatic merge of the change +by clicking the gray ``Enable auto-merge (squash)`` button. You will find it at the bottom of the pull request page. The auto-merge will only happen if all the required checks pass, but the PR does not need to have been approved for a successful auto-merge to take place. diff --git a/getting-started/pull-request-lifecycle.rst b/getting-started/pull-request-lifecycle.rst index e2dde81f67..1c16f1592d 100644 --- a/getting-started/pull-request-lifecycle.rst +++ b/getting-started/pull-request-lifecycle.rst @@ -41,8 +41,8 @@ Here is a quick overview of how you can contribute to CPython: #. `Create Pull Request`_ on GitHub to merge a branch from your fork -#. Make sure the continuous integration checks on your Pull Request - are green (i.e. successful) +#. Make sure the :ref:`continuous integration checks on your Pull Request + are green ` (i.e. successful) #. Review and address `comments on your Pull Request`_ @@ -472,7 +472,7 @@ code and leave comments in the pull request or issue tracker. 'merge-ready', you should always make sure the entire test suite passes. Leaving a pull request review on GitHub -======================================= +--------------------------------------- When you review a pull request, you should provide additional details and context of your review process. @@ -487,14 +487,55 @@ Instead of simply "approving" the pull request, leave comments. For example: #. Comment on what is "good" about the pull request, not just the "bad". Doing so will make it easier for the PR author to find the good in your comments. +#. Look at any failures in CI on the current PR. See :ref:`"Keeping CI green" + ` below for simple things you can do to help move the PR forward. + Dismissing review from another core developer -============================================= +--------------------------------------------- A core developer can dismiss another core developer's review if they confirmed that the requested changes have been made. When a core developer has assigned the PR to themselves, then it is a sign that they are actively looking after the PR, and their review should not be dismissed. +.. _keeping-ci-green: + +Keeping continuous integration green +==================================== + +Our change management workflows generally won't allow merging PRs with +failures. Therefore, if you see a CI failure on a PR, have a look +what it is about. + +Usually the failure will be directly related to the changes in the current +PR. If you happen to have any insight into the failure, let the author know +in a review comment. CI runs sometimes generate thousands of lines of output. +Even something as simple as finding the traceback and putting it in the +comment will be helpful to the PR author. + +If the failure doesn't look related to the change you're looking at, check +if it's not present on the `Release Status`_ Buildbot dashboard as well. +If so, that means the failure was introduced in a prior change. Using Buildbot's +UI you can find which PR introduced the issue and comment that it +affects other PRs. + +If you still don't see where the failure originates from, check for +a "This branch is out-of-date with the base branch" sign next to the +list of executed checks. Clicking "Update branch" next to this message +will merge in the latest changes from the base branch into the PR. + +If this still doesn't help with the failure on the PR, you can try +to re-run that particular failed check. Go to the red GitHub Action job, +click on the "Re-run jobs" button on the top right, and select +"Re-run failed jobs". The button will only be present when all other jobs +finished running. + +Re-running failed jobs shouldn't be your first instinct but it is occasionally +helpful because distributed systems can have intermittent failures, and +some of our unit tests are sensitive to overloaded virtual machines. +If you identify such flaky behavior, look for an issue in the `issue tracker`_ +that describes this particular flakiness. Create a new issue if you can't +find one. Committing/rejecting ==================== @@ -522,3 +563,4 @@ accepts your pull request. .. _issue tracker: https://github.com/python/cpython/issues .. _Core Development Discourse category: https://discuss.python.org/c/core-dev/23 +.. _Release Status: https://buildbot.python.org/all/#/release_status diff --git a/triage/triaging.rst b/triage/triaging.rst index 89919450d8..1d9e8809fe 100644 --- a/triage/triaging.rst +++ b/triage/triaging.rst @@ -94,6 +94,7 @@ you can help by making sure the pull request: * includes a :ref:`NEWS entry ` (if needed) * includes the author in ``Misc/ACKS``, either already or the patch adds them * doesn't have conflicts with the ``main`` branch +* :ref:`doesn't have failing CI checks ` Doing all of this allows core developers and :ref:`triagers ` to more quickly look for subtle issues that only people with extensive