[PATCH] D99305: [docs] Document our norms around reverts

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 333405.
reames added a comment.

Address Mehdi's comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99305/new/

https://reviews.llvm.org/D99305

Files:
  llvm/docs/CodeReview.rst
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===================================================================
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -298,6 +298,73 @@
 reminding the contributor of this policy over reverting. Minor corrections and
 omissions can be handled by sending a reply to the commits mailing list.
 
+.. _revert_policy:
+
+Reverts
+-------
+
+As a community, we strongly value having the tip of tree in a good state.  As
+such, we tend to make much heavier use of reverts than some other open source
+projects, and our norms are a bit different.
+
+When should you revert your own change?
+
+* Any time you learn of a serious problem with a change, you should revert it.
+  We strongly encourage "revert to green" as opposed to "fixing forward".  We
+  encourage reverting first, investigating offline, and then reapplying the
+  fixed patch - possibly after another round of review if warranted.
+* If you break a buildbot in a way which can't be quickly fixed, please revert.
+* If a test case is reported in the commit thread which demonstrates a problem
+  please revert, and investigate offline.
+* If you receive substantial :ref:`post-commit review <post_commit_review>`
+  feedback, please revert and address said feedback before recommitting.
+  (Possibly after another round of review.)
+* If you are asked to revert by another contributor, please revert and discuss
+  the merits of the request offline (unless doing so would further destabilize
+  tip of tree).
+
+When should you revert someone else's change?
+
+* In general, if the author themselves would revert the change per these
+  guidelines, we encourage other contributors to do so as a favor to the
+  author.  This is one of the major cases where our norms differ from others;
+  we generally consider reverting someone's change to be a favor to them.  We
+  don't expect contributors to be always available, and the assurance that a
+  problematic patch will be reverted and we can return to it at our next
+  opportunity enables this.
+* The other case for a third-party revert is for serious norm violations.  As a
+  general rule, you should never be reverting a patch for this unless you've
+  already started a discussion highlighting the problem on the original commit
+  thread.  There are exceptions where a rapid revert for a norm violation may
+  be called for, but if those are relevant for you, you already know.
+
+What are the expectations around a revert?
+
+* You should be sure that reverting the change improves the stability of tip
+  of tree.  Sometimes reverting one change in a series can worsen things
+  instead of improving them.  We expect reasonable judgment to ensure that
+  the proper patch or set of patches is being reverted.
+* You should have a publicly reproducible test case ready to share.  It is
+  not considered reasonable to revert without at least the promise to produce
+  a public test case in the near term.  We encourage sharing of test cases in
+  commit threads, or in PRs.  We encourage the reverter to minimize the test
+  case and to prune dependencies where practical.
+* Reverts should be reasonably timely.  A change submitted two hours ago
+  can be reverted by a third-party without prior discussion.  A change
+  submitted two years ago probably shouldn't be.  Where exactly the transition
+  point is is hard to say, but it's probably in the handful of days in tree
+  territory.    If you are unsure, we encourage you to reply to the commit
+  thread, give the author a bit to respond, and then proceed with the revert
+  if the author doesn't seem to be actively responding.
+
+How should you respond if someone reverted your change?
+
+* In general, the appropriate response is to start by thanking them.  If you
+  need more information to address the problem, please follow up in the
+  original commit thread with the reverting patch author.
+* It is normal and healthy to have patches reverted.  Having a patch reverted
+  does not necessarily mean you did anything wrong.
+
 Obtaining Commit Access
 -----------------------
 
Index: llvm/docs/CodeReview.rst
===================================================================
--- llvm/docs/CodeReview.rst
+++ llvm/docs/CodeReview.rst
@@ -36,13 +36,15 @@
 responsible for making all necessary review-related changes, including
 those requested during any post-commit review.
 
+.. _post_commit_review:
+
 Can Code Be Reviewed After It Is Committed?
 -------------------------------------------
 
 Post-commit review is encouraged, and can be accomplished using any of the
 tools detailed below. There is a strong expectation that authors respond
 promptly to post-commit feedback and address it. Failure to do so is cause for
-the patch to be reverted.
+the patch to be :ref:`reverted <revert_policy>`.
 
 If a community member expresses a concern about a recent commit, and this
 concern would have been significant enough to warrant a conversation during


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99305.333405.patch
Type: text/x-patch
Size: 4998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210325/9c8192cd/attachment.bin>