Skip to content

Commit 62000c9

Browse files
committed
Add revert policy
This is based on an [old discussion in #t-compiler][compiler] as well as [a more recent one in #t-release][release]. The outcome of both discussions was that a clearer revert policy should be documented that was biased toward reverting PRs that caused regressions. [compiler]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/to.20revert.20or.20not.20to.20revert.20that.20is.20the.20question [release]: https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/meeting.202022.2010.2028/near/306693752
1 parent d38b475 commit 62000c9

File tree

2 files changed

+78
-0
lines changed

2 files changed

+78
-0
lines changed

src/compiler/revert-button.png

20.6 KB
Loading

src/compiler/reviews.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,83 @@ 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 regression, the best policy
33+
is to revert it quickly and re-land it later once a fix and regression test are
34+
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 "forward fix" a regression with a follow-up PR. However,
99+
if real-world users have reported being affected, this practice is strongly
100+
discouraged unless a high-confidence fix is already in the bors queue.
101+
102+
While it can feel like a significant step backward to have your PR reverted, in
103+
most cases it is much easier to land the PR a second time once a fix can be
104+
confirmed. Allowing a revert to land takes pressure off of you and your
105+
reviewers to act quickly and gives you time to address the issue fully.
106+
30107
## Rollups
31108

32109
All reviewers are strongly encouraged to explicitly mark a PR as to whether or
@@ -82,6 +159,7 @@ The following is some guidance for setting priorities:
82159
- 1-5
83160
- P-high issue fixes
84161
- Toolstate fixes
162+
- Reverts containing the above
85163
- Beta-nominated PRs
86164
- Submodule/Subtree updates
87165
- 5+

0 commit comments

Comments
 (0)