[PATCH] D89995: Make the post-commit review expectations more explicit with respect to revert

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle requested changes to this revision.
nhaehnle added a reviewer: arsenm.
nhaehnle added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: wdng.

Considering that you are doing this in response to D83088 <https://reviews.llvm.org/D83088>, I think we have pre-existing evidence that this isn't workable as-is. There **has** to be some obligation on the people "thinking there should be more review" to actually engage in review. In particular, if they have previously demonstrated that they cannot be relied on for this, then the rule should not apply.

I'm not sure how to phrase that and whether the phrasing is even necessary. In particular, I doubt that this distinction is necessary if we were to lower the bar as I've explained inline.



================
Comment at: llvm/docs/CodeReview.rst:47-48
 
-In addition, if substantial problems are identified, it is expected that the
-patch is reverted and fixed offline. Before being recommitted, the patch
-generally undergoes further review, including by the community member who
-identified the problem and, in cases where the patch triggered a
-hardware-specific buildbot failure, a community member with access to hardware
-similar to that on the buildbot that the patch previously caused to fail.
+If a developer thinks that a recently committed patch would benefit from some
+more review, the patch should be reverted promptly (preferably by the original
+author, unless they are unresponsive). Developers often disagree, and erring on
----------------
This is too low a bar for reverting given how unresponsive reviewers are in practice. I would suggest instead:

> If a developer **identifies a problem** in a recently committed patch, the patch should reverted promptly ...

This also makes more sense because it aligns with the second paragraph, which talks about "the community member who **identified the problem**".

You can always think that more review is better, but unless there is something **concrete**, we err too much on the side of review limbo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89995