Skip to content

Commit 5b23a62

Browse files
authored
Merge pull request #663 from tmandry/revert-policy
Add revert policy
2 parents d38b475 + 4a975c3 commit 5b23a62

File tree

2 files changed

+88
-0
lines changed

2 files changed

+88
-0
lines changed

src/compiler/revert-button.png

20.6 KB
Loading

src/compiler/reviews.md

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,93 @@ delegate+` or `@bors delegate=username`. This will allow the PR author
2727
to approve the PR by issuing `@bors` commands like the ones above
2828
(but this privilege is limited to the single PR).
2929

30+
## Reverts
31+
32+
If a merged PR is found to have caused a meaningful unanticipated regression,
33+
the best policy is to revert it quickly and re-land it later once a fix and
34+
regression test are added.
35+
36+
A "meaningful regression" in this case is up to the judgment of the person
37+
approving the revert. As a rule of thumb, this would be a bug in a stable
38+
or otherwise important feature that causes code to stop compiling, changes
39+
runtime behavior, or triggers a (warn-by-default or higher) lint incorrectly in
40+
real-world code.
41+
42+
When these criteria are in doubt, and especially if real-world code is affected,
43+
revert the PR. This allows bleeding edge users to continue to use and report
44+
bugs on HEAD with a higher degree of certainty about where new bugs are introduced.
45+
46+
Before being reverted, a PR should be shown to cause a regression with a fairly
47+
high degree of certainty (e.g. bisection on commits, or bisection on nightlies
48+
with one or more compiler team members pointing to this PR, or it's simply
49+
obvious to everyone involved). Only revert with lower certainty if the issue is
50+
particularly critical or urgent to fix.
51+
52+
### Creating reverts
53+
54+
The easiest method for creating a revert is to use the "Revert" button on
55+
Github. This appears next to the "bors merged commit abcd" message on a pull
56+
request, and creates a new pull request.
57+
58+
![Location of the "Revert" button](revert-button.png)
59+
60+
Alternatively, a revert commit can be created using the git CLI and then
61+
uploaded as a pull request:
62+
63+
```terminal
64+
$ git revert -m 1 62d5bee
65+
```
66+
67+
It's polite to tag the author and reviewer of the original PR so they know
68+
what's going on. You can use the following message template:
69+
70+
```
71+
Reverts rust-lang/rust#123456
72+
cc @author @reviewer
73+
74+
This revert is based on the following report of a regression caused by this PR:
75+
<link to issue or comment(s)>
76+
77+
In accordance with the compiler team [revert policy], PRs that cause meaningful
78+
regressions should be reverted and re-landed once the regression has been fixed
79+
(and a regression test has been added, where appropriate).
80+
[revert policy]: https://forge.rust-lang.org/compiler/reviews.html#reverts
81+
82+
Fear not! Regressions happen. Please rest assured that this does not
83+
represent a negative judgment of your contribution or ability to contribute
84+
positively to Rust in the future. We simply want to prioritize keeping existing
85+
use cases working, and keep the compiler more stable for everyone.
86+
87+
r? compiler
88+
```
89+
90+
If you have r+ privileges, you can self-approve a revert.
91+
92+
Generally speaking, reverts should have elevated priority and match the `rollup`
93+
status of the PR they are reverting. If a non-rollup PR is shown to have no
94+
impact on performance, it can be marked `rollup=always`.
95+
96+
### Forward fixes
97+
98+
Often it is tempting to address a regression by posting a follow-up PR that,
99+
rather than reverting the regressing PR, instead augments the original in
100+
small ways without reverting its changes overall. However, if real-world users
101+
have reported being affected, this practice is strongly discouraged unless one
102+
of the following is true:
103+
104+
* A high-confidence fix is already in the bors queue.
105+
* The regression has made it to a release branch (beta or stable) and a
106+
[backport] is needed. Often the "smallest possible change" is desired for a
107+
backport. The offending PR may or may not still be reverted on the main
108+
branch; this is left to the discretion of someone who can `r+` it.
109+
110+
[backport]: ../release/backporting.md
111+
112+
While it can feel like a significant step backward to have your PR reverted, in
113+
most cases it is much easier to land the PR a second time once a fix can be
114+
confirmed. Allowing a revert to land takes pressure off of you and your
115+
reviewers to act quickly and gives you time to address the issue fully.
116+
30117
## Rollups
31118

32119
All reviewers are strongly encouraged to explicitly mark a PR as to whether or
@@ -82,6 +169,7 @@ The following is some guidance for setting priorities:
82169
- 1-5
83170
- P-high issue fixes
84171
- Toolstate fixes
172+
- Reverts containing the above
85173
- Beta-nominated PRs
86174
- Submodule/Subtree updates
87175
- 5+

0 commit comments

Comments
 (0)