[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

Thu Apr 9 21:25:05 PDT 2020

dblaikie added a comment.

In D77683#1973757 <https://reviews.llvm.org/D77683#1973757>, @jdoerfert wrote:

> In D77683#1970826 <https://reviews.llvm.org/D77683#1970826>, @mehdi_amini wrote:
>
> > I am still not sure what "if someone has asked for extra review of a specific area" refers to?
>
>
> As said earlier
>
> >>   If I understand this correctly, this is meant to cover situations where reviewers are active in an area and indicated an interest in reviewing basically everything.
> > 
> > Pretty much, yes.
>
> this should mean that, if requested, all non-trivial patches should go through review. The current wording is very lenient, especially wrt. code owners.
>  While most people I talked to don't see owners as special per se (but just assume they have more responsibility), this paragraph says they have special rights.
>  Given the murky ways owners are "selected", I think we should have a well defined way to limit these rights without revoking the status.

I disagree here. I think it's suitable (within the way the LLVM project works) for code owners to commit without review - given they are the arbiters of what's acceptable in that subcomponent - ultimately they can veto anyone else (short of a broader project/code owner). Doesn't usually come to that, but that's what the ownership role means.

I don't think it's correct for arbitrary contributors to say "you need to/this component needs review-before-commit" - the code owner could say that if they really don't trust any of the contributors to conform to the direction they have in mind for the component (that's not to discredit the contributors - but that's the point of pre-commit review: to ensure it conforms to the code owners vision for the component).

(yes, code owners aren't meant to be dictators - but they're ultimately the final decider)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77683