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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast resigned from this revision.
hubert.reinterpretcast added a comment.

I have just some typo/phrasing fixes to suggest. I think my other concerns have been addressed.



================
Comment at: llvm/docs/DeveloperPolicy.rst:318-319
+* 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>`
----------------
I really do think that at least the comma should be moved. I also suggest changing the ordering to clarify that the "test case" (as opposed to the thread) is the subject of the clause.


================
Comment at: llvm/docs/DeveloperPolicy.rst:350
+* The commit message for the reverting commit should explain why patch
+  is being reverted.  It is also customary to response to the original
+  commit email mentioning the revert.
----------------
Fix typo.


================
Comment at: llvm/docs/DeveloperPolicy.rst:362
+  requires hardware patch author doesn't have access to, sharp regression in
+  compile time of internal workload, etc..), the reverter is expected to be
+  strongly proactive about working with the patch author to debug and test
----------------
Suggest not to have two periods for `etc.`.


================
Comment at: llvm/docs/DeveloperPolicy.rst:323
+* 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).
----------------
reames wrote:
> hubert.reinterpretcast wrote:
> > reames wrote:
> > > hubert.reinterpretcast wrote:
> > > > This point along with the "if the author themselves would revert" below means that unilateral reverts without discussion is being encouraged here. I'm not in favour of action without discussion.
> > > This is our long standing practice.  
> > Sure, but probably for specific reasons and not for all. Otherwise, there wouldn't be "requests" to revert at all (except from people with no commit access).
> Again, long standing practice.  I'm explicitly not changing anything, I'm just documenting.  I ask you move this point to the llvm-dev thread for broader discussion and possibly a follow up patch if warranted.  
In conjunction with the expanded room for discretion below, I guess this works.


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

https://reviews.llvm.org/D99305