[PATCH] D99305: [docs] Document our norms around reverts
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 26 11:27:55 PDT 2021
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
More information about the llvm-commits
mailing list