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

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 26 03:01:11 PDT 2020


rengolin added a comment.

In D89995#2352053 <https://reviews.llvm.org/D89995#2352053>, @nhaehnle wrote:

> 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.

I don't seem to be reading D83088 <https://reviews.llvm.org/D83088> in the same light as you. To me, this proposal would have clearly solved that problem. David was the main reviewer and has given an extensive explanation on the issues he was still seeing with the patch that landed. This is as clear as it comes, that patch must be reverted as soon as possible.

> 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 do not understand what you see as a demonstration of lack of reliability is but I see none there. And I don't see it often in LLVM. Certainly not from David! And that review was no example of that either.

Plus, writing rules on vague concepts of reliability that are clearly interpreted by different people differently makes for bad policy.

Historically, all our policies err in the side of trust. We trust the reviewers have good intention and we trust authors will act upon it the best they can. I honestly would hate to see us moving into a position where we are second-guessing everyone's intentions. It doesn't scale.

> 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.

I agreed with the bar being low in your inline comment and Mehdi has fixed that. Please read again the text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89995



More information about the llvm-commits mailing list