Skip to content

Commit

Permalink
Add section on keeping CI green (#1215)
Browse files Browse the repository at this point in the history
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>
  • Loading branch information
4 people committed Feb 6, 2024
1 parent 1a56667 commit b5f49de
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
8 changes: 5 additions & 3 deletions getting-started/git-boot-camp.rst
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,11 @@ You can read more about what to look for before accepting a change
:ref:`here <committing>`.

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" <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.
Expand Down
50 changes: 46 additions & 4 deletions getting-started/pull-request-lifecycle.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <keeping-ci-green>` (i.e. successful)

#. Review and address `comments on your Pull Request`_

Expand Down Expand Up @@ -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.
Expand All @@ -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"
<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
====================
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions triage/triaging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ you can help by making sure the pull request:
* includes a :ref:`NEWS entry <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 <keeping-ci-green>`

Doing all of this allows core developers and :ref:`triagers <triage-team>`
to more quickly look for subtle issues that only people with extensive
Expand Down

0 comments on commit b5f49de

Please sign in to comment.