From 8fd91dd9356969b5798fe04654842244dcc9d8b5 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 20 Jan 2026 16:02:19 +0100 Subject: [PATCH 1/6] Expand guidance on failing status checks --- CONTRIBUTING.md | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b1753c..657efc3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -639,29 +639,25 @@ to verify the code works as expected. If PRs are merged with failing status checks, there are potential bugs in the code which may lead to downtime or other operational issues. -PRs can only be merged if all status checks pass. Some exceptions could -include: +PRs should only be merged if all status checks pass. Changes should only be landed +if are urgently needed in production and the test suit of the existing charm is +failing for spurious reasons. -* If the team has recently taken over a new charm and changes are urgently - needed and the test suit of the existing charm is failing for spurious - reasons. -* A tool we depend on is not working, no previous working version is available - and a change is urgently needed to fix a critical issue in production. - -Even in the above cases it is not clear whether it is reasonable to merge a PR +Even in the above case it is not clear whether it is reasonable to merge a PR with failing checks due to the high risks this entails. Judgement is required in these cases weighing the risks of introducing bugs with the urgency and impact of the underlying need to land the change. -Alternatives to merging the PR with failing status checks include: +Before merging the PR with failing status checks evaulate the following options: * Change the code to fix the problem. +* Re-run the test in case of suspected flakiness. +* Wait for an upstream fix for the issue. * Disable the status check (e.g., mark the test as [`xfail`](https://docs.pytest.org/en/7.1.x/how-to/skipping.html)). This should not be done lightly as the value the status check provides to the team - is lost. -* Wait for an upstream fix for the issue. + is lost and tracking disabled test is challenging. If it is deemed that a change should land despite a failing status check, the following artifacts should be added to the PR: @@ -673,9 +669,7 @@ following artifacts should be added to the PR: (e.g., because the fix is needed in production and the status check is failing due to problems with GitHub). -One of the repository admins should then be asked to review the changes and -merge the PR after completing the review. The PR should be otherwise ready to be -merged (e.g., has been approved). +The PR should be otherwise ready to be merged (e.g., has been approved). Resolving the underlying reason the status check is failing should be high priority so that the team can rely on the automation again. From 826d536e03b9cc6f5cb83c53766051167fa00d71 Mon Sep 17 00:00:00 2001 From: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> Date: Wed, 21 Jan 2026 09:28:25 +0100 Subject: [PATCH 2/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Georget --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 657efc3..a440e7c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -649,7 +649,7 @@ Judgement is required in these cases weighing the risks of introducing bugs with the urgency and impact of the underlying need to land the change. -Before merging the PR with failing status checks evaulate the following options: +Before merging the PR with failing status checks evaluate the following options: * Change the code to fix the problem. * Re-run the test in case of suspected flakiness. From 4591945ef49de5c16d56a8fd8632a174d3d304ee Mon Sep 17 00:00:00 2001 From: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> Date: Wed, 21 Jan 2026 15:11:41 +0100 Subject: [PATCH 3/6] Update CONTRIBUTING.md Co-authored-by: Erin Conley --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a440e7c..f269168 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -640,7 +640,7 @@ checks, there are potential bugs in the code which may lead to downtime or other operational issues. PRs should only be merged if all status checks pass. Changes should only be landed -if are urgently needed in production and the test suit of the existing charm is +if are urgently needed in production and the test suite of the existing charm is failing for spurious reasons. Even in the above case it is not clear whether it is reasonable to merge a PR From 521157262ac7626dccd1d09a73b38820298318e9 Mon Sep 17 00:00:00 2001 From: arturo-seijas <102022572+arturo-seijas@users.noreply.github.com> Date: Wed, 21 Jan 2026 15:11:49 +0100 Subject: [PATCH 4/6] Update CONTRIBUTING.md Co-authored-by: Erin Conley --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f269168..a02da18 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -657,7 +657,7 @@ Before merging the PR with failing status checks evaluate the following options: * Disable the status check (e.g., mark the test as [`xfail`](https://docs.pytest.org/en/7.1.x/how-to/skipping.html)). This should not be done lightly as the value the status check provides to the team - is lost and tracking disabled test is challenging. + is lost and tracking a disabled test is challenging. If it is deemed that a change should land despite a failing status check, the following artifacts should be added to the PR: From 358417e76b5fa6220e11575a0a7c987a8884d3f9 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 21 Jan 2026 15:14:46 +0100 Subject: [PATCH 5/6] clarify --- CONTRIBUTING.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a02da18..56a3c3e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -640,7 +640,7 @@ checks, there are potential bugs in the code which may lead to downtime or other operational issues. PRs should only be merged if all status checks pass. Changes should only be landed -if are urgently needed in production and the test suite of the existing charm is +if are urgently needed in production and the test suite of the existing project is failing for spurious reasons. Even in the above case it is not clear whether it is reasonable to merge a PR @@ -654,6 +654,7 @@ Before merging the PR with failing status checks evaluate the following options: * Change the code to fix the problem. * Re-run the test in case of suspected flakiness. * Wait for an upstream fix for the issue. +* Remove the test if it doesn't add value. * Disable the status check (e.g., mark the test as [`xfail`](https://docs.pytest.org/en/7.1.x/how-to/skipping.html)). This should not be done lightly as the value the status check provides to the team @@ -671,6 +672,8 @@ following artifacts should be added to the PR: The PR should be otherwise ready to be merged (e.g., has been approved). +Note that only repository admins will be able to force merge pull requests. + Resolving the underlying reason the status check is failing should be high priority so that the team can rely on the automation again. From be4193f0aabbac361c34f2d9dfaaf1a620229052 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 21 Jan 2026 15:17:50 +0100 Subject: [PATCH 6/6] examples --- CONTRIBUTING.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56a3c3e..1f788a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -640,10 +640,14 @@ checks, there are potential bugs in the code which may lead to downtime or other operational issues. PRs should only be merged if all status checks pass. Changes should only be landed -if are urgently needed in production and the test suite of the existing project is -failing for spurious reasons. - -Even in the above case it is not clear whether it is reasonable to merge a PR +if are urgently needed in production and the failures can't be fixed in a reasonable +timeline. Some examples include: +* If the team has recently taken over a new charm and changes are urgently +needed and the test suit of the existing charm is failing for spurious reasons. +* A tool we depend on is not working, no previous working version is available +and a change is urgently needed to fix a critical issue in production. + +Even in the above cases it is not clear whether it is reasonable to merge a PR with failing checks due to the high risks this entails. Judgement is required in these cases weighing the risks of introducing bugs with the urgency and impact of the underlying need to