Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -639,29 +639,30 @@ 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 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.
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.
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
land the change.

Alternatives to merging the PR with failing status checks include:
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
is lost.
* Wait for an upstream fix for the issue.
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:
Expand All @@ -673,9 +674,9 @@ 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).

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.
Expand Down