diff --git a/getting-started/fixing-issues.rst b/getting-started/fixing-issues.rst index f277cbf60..8dad09484 100644 --- a/getting-started/fixing-issues.rst +++ b/getting-started/fixing-issues.rst @@ -16,10 +16,59 @@ For the truly adventurous looking for a challenge, you can look for issues that are not considered easy and try to fix those. It must be warned, though, that it is quite possible that a bug that has been left open has been left into that state because of the difficulty compared to the benefit of the fix. It could -also still be open because no consensus has been reached on how to fix the -issue, although having a pull request that proposes a fix can turn the tides of the -discussion to help bring it to a close. Regardless of why the issue is open, +also still be open because nobody agreed on how to fix it (see +:ref:`fixing-issues-readiness`). Regardless of why the issue is open, you can also always provide useful comments if you do attempt a fix, successful or not. + +.. _fixing-issues-readiness: + +Check that the issue is ready to be worked on +============================================= + +An obvious bug that nobody is working on can be fixed directly, but most +other issues have to be confirmed as worth fixing first, as they may be +duplicates, feature requests filed as bugs, already rejected features, etc. +A genuine bug is not always fixed either, since the fix may break existing +code and its impact has to be weighed. In that case, wait until a triager +or a core team member has acknowledged the issue. + +If you are unsure, ask in the issue, but avoid superfluous comments, since +every comment notifies everyone following the issue. Issues are not +assigned to contributors who ask for it, but once an issue has been +acknowledged and is still relevant, you can start working on it and +mention it in the issue. + +Before opening a pull request, read the whole discussion and check that the +issue still applies to the latest state of the branch you are targeting, +usually ``main``: an acknowledged issue may have been fixed since, or the +code it describes may have changed. For old issues, reporting that the +behaviour can no longer be reproduced on a supported Python version (see +:ref:`branchstatus`) is helpful on its own. + +In general, avoid opening a pull request if: + +* the issue has the :gh-label:`pending` label; +* the issue is old and its discussion never reached consensus; +* someone, such as the issue's author, already said they would open one; +* a pull request for the issue already exists. + +In the last two cases, if there has been no activity for a while, ask the +contributor whether they still want to work on it. + +Do not use automated tools to open pull requests on new issues either: a +pull request on an untriaged issue costs triagers more time than the issue +alone (see :ref:`ai-tools`). + + +.. _fixing-issues-no-pullrequest: + +Consider whether a pull request is needed +========================================= + +Before writing a pull request, check that the report describes an actual +bug (see :ref:`issue-tracker-not-a-bug`). + + .. _"easy" issues: https://github.com/python/cpython/issues?q=is%3Aissue+is%3Aopen+label%3Aeasy diff --git a/getting-started/pull-request-lifecycle.rst b/getting-started/pull-request-lifecycle.rst index 3b8e09ae1..cb39c506a 100644 --- a/getting-started/pull-request-lifecycle.rst +++ b/getting-started/pull-request-lifecycle.rst @@ -20,6 +20,33 @@ For a cheat-sheet reference, see the :ref:`quick reference on creating pull requests `. +.. _pullrequest-prerequisites: + +Before writing code +=================== + +Most changes start with an issue, and the pull request usually comes once +the issue has been acknowledged (see :ref:`fixing-issues-readiness`), +although an issue does not always need one (see +:ref:`fixing-issues-no-pullrequest`). Small features can be proposed +directly in an issue, but features that break existing APIs and larger +changes, such as changes to protocols or built-ins, usually need a +discussion in the `Ideas category on Discourse`_ first (see +:ref:`langchanges` and :ref:`stdlibchanges`). + +Performance improvements should always be proposed in an issue first, +ideally without a pull request, since they need to be discussed unless the +gain is clear. Keep any proof of concept on a branch of your fork instead +of opening a pull request for it. We strongly recommend including the +benchmark results in the issue, together with the benchmark script and the +configuration used. Use ``pyperf timeit`` from :pypi:`pyperf` instead of +plain :mod:`timeit`, or ``-X importtime`` for import time. Optimizing a +cold path, or a rare case at the cost of increased complexity, is usually +not worth it, so base the case for the change on realistic usage. + +.. _Ideas category on Discourse: https://discuss.python.org/c/ideas/6 + + .. _pullrequest-steps: Step-by-step guide @@ -233,6 +260,12 @@ For the last two, note the following: change and the original** ``NEWS`` **entry remains valid**, then no additional entry is needed. +The bot adds the :gh-label:`skip news` label by itself in some cases, for +example when a pull request touches nothing but documentation. If your +change is one of the exceptions above and the NEWS check still fails, don't +add an entry and wait for a triager or a core team member to add the label +for you. + Changes that require "What's New in Python" entries --------------------------------------------------- @@ -387,14 +420,18 @@ Typo fixes Fixing typos and grammatical errors in documentation is a contribution that does not require a linked issue. The most appropriate -place for such fixes are the :cpy-file:`Doc/` (end-user documentation) -and :cpy-file:`InternalDocs/` directories. +place for such fixes are the :cpy-file:`Doc/` (end-user documentation), +:cpy-file:`InternalDocs/` directories, or Python and C docstrings. -Keep typo-fix PRs **small and focused**. Large PRs that touch many unrelated files +Keep typo-fix PRs **small and focused**, and try to fix all the typos of a +file or topic, including the matching docstrings, in a single PR rather +than one by one. Where possible, avoid reflowing the surrounding lines, so +that the diff only shows the fixes. Large PRs that touch many unrelated files notify a large number of reviewers unnecessarily and are harder to review. -Large typo-fix PRs, or PRs touching directories outside those listed above, -may be closed with a reference to this section. +Large typo-fix PRs, PRs touching directories outside those listed above, or +PRs fixing typos in text users never see like code comments may be closed +with a reference to this section. .. _patchcheck: diff --git a/getting-started/quick-reference.rst b/getting-started/quick-reference.rst index 909e2f41a..6b2f90fc2 100644 --- a/getting-started/quick-reference.rst +++ b/getting-started/quick-reference.rst @@ -100,7 +100,8 @@ Create issues for nontrivial changes For most changes, `create an issue `__ before submitting a pull request. -Trivial changes like typo fixes do not need issues. +Trivial changes like typo fixes or that only increase test coverage do not +need issues. Create work branches -------------------- diff --git a/triage/issue-tracker.rst b/triage/issue-tracker.rst index c730a0218..8b9f93e80 100644 --- a/triage/issue-tracker.rst +++ b/triage/issue-tracker.rst @@ -47,6 +47,26 @@ above the list of bugs on the issues page. See :ref:`searching-gh-issues` for more information. +.. _issue-tracker-not-a-bug: + +Checking if the behaviour is a bug +---------------------------------- + +The following reports are usually closed without a fix: + +* calling a function in a way that violates its documented contract. This + is *garbage in, garbage out* (GIGO), and a wrong value or an unexpected + exception is generally not considered a bug; +* bugs found through private APIs, unless realistic code using public APIs + could trigger them as well; +* crashes caused by manipulating the interpreter's internals with tools + such as :meth:`codeobject.replace` or :mod:`ctypes`; +* changes that bring code churn without fixing an actual problem, for + instance changing whitespace and other stylistic details for the sake of + a foolish consistency. Maintainers sometimes make such changes + themselves. + + Creating a new issue --------------------