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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
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>`
----------------
reames wrote:
> hubert.reinterpretcast wrote:
> > 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.
> I've reread this sentence a couple of times, and don't see your point.  Can you quote the exact change you're suggesting?
> 
> (I was never great at formal english grammar so I might be misparsing something.)
I did, but maybe it's not showing for you?

My suggested rewrite is:
If a test case that demonstrates a problem is reported in the commit thread, please revert and investigate offline.

That is, I moved the comma to before the "please". I also moved the "demonstrates a problem" part to right after "test case" instead of being after "commit thread".


================
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
----------------
reames wrote:
> hubert.reinterpretcast wrote:
> > Suggest not to have two periods for `etc.`.
> This is an ellipsis to indicate the list is not complete.  I believe this is correct usage.  Some sources say it must be three periods (not two), but I see both in regular usage.  
I've never heard of two period ellipses being considered formally correct. I think `etc.` is enough to indicate that the list is not complete.


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

https://reviews.llvm.org/D99305