From d140165f44962eebd05a6de6f751925116c1403b Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 9 Apr 2022 17:42:37 +0200 Subject: [PATCH 1/7] Allow triagers to close PRs if the PR is of very low quality As discussed on the CPython core-dev discord channel, there are several issues of the current policy that states that triagers should never close PRS: - Github's setup allows members of the triage team to close PRs, regardless of the policy stated in the devguide. - This policy isn't consistently followed by all members of the triage team. Some triagers occasionally close PRs (I have closed one or two myself in situations where it seemed abundantly clear that there was no prospect of the PR being merged). - This policy isn't really enforced in any way if triagers do close PRs. This lack of enforcement seems to be an implicit endorsement of the idea that it is okay for triagers to close PRs in some circumstances. - It feels silly to have to bother a core dev with a request for a PR to be closed if it's self-evident that it should be closed (e.g. if the PR makes changes to a deprecated module, makes solely cosmetic changes to a module, or simply has a 0% chance of ever being merged) - Changing this policy will help towards the goal of reducing the CPython PR backlog --- triaging.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/triaging.rst b/triaging.rst index 43549bae4..ce09caa97 100644 --- a/triaging.rst +++ b/triaging.rst @@ -35,17 +35,23 @@ Responsibilities include: - Good first issue - Other categorizations -As triagers gain experience, they may have some intuition of when a PR should -be closed. Triagers can recommend closing a PR, but the final decision must be -made by a core developer. By having triagers and core developers work together, +Although triagers have the power to close PRs, they should generally not do so +without first consulting a core developer. By having triagers and core developers work together, the author receives a careful consideration of their PR. This encourages future contributions, regardless of whether their PR is accepted or closed. -Triagers can make use of the ``invalid`` and ``stale`` labels to suggest that a +Nonetheless, triagers should feel free to close a PR if they judge that the +chance of the PR being merged is exceedingly low. This can include PRs that +make solely cosmetic changes to a module, PRs that propose changes to +deprecated modules, and/or PRs that self-evidently lie far below the quality +threshhold required for a change to be made to code in the Python GitHub +repositories. + +Triagers can also make use of the ``invalid`` and ``stale`` labels to suggest that a PR may be suitable for closure. For more information, see the :ref:`GitHub PR labels ` section. -It is also of paramount importance to treat every contributor to the Python +Note that it is of paramount importance to treat every contributor to the Python project kindly and with respect. Regardless of whether they're entirely new or a veteran core developer, they're actively choosing to voluntarily donate their time towards the improvement of Python. As is the case with any member of From 05d5ef967971315e4a69d3d558d71ac0f6e246a0 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 10 Apr 2022 00:20:16 +0200 Subject: [PATCH 2/7] Address reviews --- triaging.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/triaging.rst b/triaging.rst index ce09caa97..ca379e0e8 100644 --- a/triaging.rst +++ b/triaging.rst @@ -41,11 +41,15 @@ the author receives a careful consideration of their PR. This encourages future contributions, regardless of whether their PR is accepted or closed. Nonetheless, triagers should feel free to close a PR if they judge that the -chance of the PR being merged is exceedingly low. This can include PRs that -make solely cosmetic changes to a module, PRs that propose changes to -deprecated modules, and/or PRs that self-evidently lie far below the quality -threshhold required for a change to be made to code in the Python GitHub -repositories. +chance of the PR being merged is exceedingly low. This can include (but is not +necessarily limited to) PRs that meet at least one of the following criteria: + +* PRs proposing solely cosmetic changes +* PRs proposing changes to deprecated modules +* PRs that are no longer relevant. This includes: + - PRs proposing fixes for bugs that can no longer be reproduced + - PRs proposing changes that have been rejected by Python core developers + elsewhere (e.g. in an issue or a PEP rejection notice). Triagers can also make use of the ``invalid`` and ``stale`` labels to suggest that a PR may be suitable for closure. For more information, see the From 053ed1152db3f2a7f5e6300c5216c7e06512a473 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 9 Apr 2022 23:29:04 +0100 Subject: [PATCH 3/7] Nit --- triaging.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triaging.rst b/triaging.rst index ca379e0e8..6eae2132f 100644 --- a/triaging.rst +++ b/triaging.rst @@ -42,7 +42,7 @@ contributions, regardless of whether their PR is accepted or closed. Nonetheless, triagers should feel free to close a PR if they judge that the chance of the PR being merged is exceedingly low. This can include (but is not -necessarily limited to) PRs that meet at least one of the following criteria: +necessarily limited to) PRs that one or more of the following criteria: * PRs proposing solely cosmetic changes * PRs proposing changes to deprecated modules From bc25768d7c19edea11b5ee4f246ab959a575798e Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 9 Apr 2022 23:29:41 +0100 Subject: [PATCH 4/7] Typo --- triaging.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triaging.rst b/triaging.rst index 6eae2132f..e9de29459 100644 --- a/triaging.rst +++ b/triaging.rst @@ -42,7 +42,7 @@ contributions, regardless of whether their PR is accepted or closed. Nonetheless, triagers should feel free to close a PR if they judge that the chance of the PR being merged is exceedingly low. This can include (but is not -necessarily limited to) PRs that one or more of the following criteria: +necessarily limited to) PRs that meet one or more of the following criteria: * PRs proposing solely cosmetic changes * PRs proposing changes to deprecated modules From 8b1256100e2d5b31036ff4200a441ef3c4abfb02 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sat, 9 Apr 2022 23:33:13 +0100 Subject: [PATCH 5/7] Errant period --- triaging.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/triaging.rst b/triaging.rst index e9de29459..e4b531024 100644 --- a/triaging.rst +++ b/triaging.rst @@ -49,7 +49,7 @@ necessarily limited to) PRs that meet one or more of the following criteria: * PRs that are no longer relevant. This includes: - PRs proposing fixes for bugs that can no longer be reproduced - PRs proposing changes that have been rejected by Python core developers - elsewhere (e.g. in an issue or a PEP rejection notice). + elsewhere (e.g. in an issue or a PEP rejection notice) Triagers can also make use of the ``invalid`` and ``stale`` labels to suggest that a PR may be suitable for closure. For more information, see the From f2166e71936834852a750d08b8a413cb8d80813a Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 10 Apr 2022 09:57:23 +0100 Subject: [PATCH 6/7] Add Jelle's suggestion, make further misc revisions --- triaging.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/triaging.rst b/triaging.rst index e4b531024..fe57aa1fc 100644 --- a/triaging.rst +++ b/triaging.rst @@ -41,8 +41,9 @@ the author receives a careful consideration of their PR. This encourages future contributions, regardless of whether their PR is accepted or closed. Nonetheless, triagers should feel free to close a PR if they judge that the -chance of the PR being merged is exceedingly low. This can include (but is not -necessarily limited to) PRs that meet one or more of the following criteria: +chance of the PR being merged would be exceedingly low, even if substantial +revisions were made to the PR. This can include (but is not necessarily +limited to) PRs that fall into one or more of the following categories: * PRs proposing solely cosmetic changes * PRs proposing changes to deprecated modules @@ -51,6 +52,9 @@ necessarily limited to) PRs that meet one or more of the following criteria: - PRs proposing changes that have been rejected by Python core developers elsewhere (e.g. in an issue or a PEP rejection notice) +If a triager has any doubt about whether to close a PR, they should consult a core +developer before taking any action. + Triagers can also make use of the ``invalid`` and ``stale`` labels to suggest that a PR may be suitable for closure. For more information, see the :ref:`GitHub PR labels ` section. From a81c52f4961f78215b3e737d5a8693f3d64b7433 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Sun, 10 Apr 2022 12:58:46 +0100 Subject: [PATCH 7/7] Update triaging.rst Co-authored-by: Hugo van Kemenade --- triaging.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/triaging.rst b/triaging.rst index fe57aa1fc..422c85dc7 100644 --- a/triaging.rst +++ b/triaging.rst @@ -42,8 +42,8 @@ contributions, regardless of whether their PR is accepted or closed. Nonetheless, triagers should feel free to close a PR if they judge that the chance of the PR being merged would be exceedingly low, even if substantial -revisions were made to the PR. This can include (but is not necessarily -limited to) PRs that fall into one or more of the following categories: +revisions were made to the PR. This includes (but is not limited to) the +following: * PRs proposing solely cosmetic changes * PRs proposing changes to deprecated modules