dblaikie added a comment. In D77683#1978070 <https://reviews.llvm.org/D77683#1978070>, @hfinkel wrote:
> In D77683#1973762 <https://reviews.llvm.org/D77683#1973762>, @dblaikie wrote: > > > 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). > > privledges > > (yes, code owners aren't meant to be dictators - but they're ultimately > > the final decider) > > > I disagree that this characterizes our conception of the role of code owner > within the project, but there is some mitigating context. In my experience, > we describe code owners as "reviewers of last resort", and their primary > responsibility is to prevent review stagnation with their component > (including from new/infrequent contributors whose patches might otherwise go > unreviewed). The code owner is not the final decider in all cases, but can > serve as a tie breaker when community consensus is unclear. That's an > important role, because it ensures our ability to make forward progress, but > is different in character from someone with overriding final authority. The > code owners vision matters only slightly if the community consensus is for a > different vision. There certainly are parts of LLVM that are developed by one > person, and that person is the code owner, and thus excessive a lot of > influence over the future direction of the component. Patches are often > contributed without pre-commit review, in this case, and the code owner is > often the most-skilled reviewer for any other patches as well. However, this > state exists only at the pleasure of the rest of the community - we all see > these patches on the commits list, and should more people become involved and > request a more-inclusive pre-commit review cycle, that's what should happen. > For components actively developed by many people, code owners have relatively > minor privileges in this regard. > > I suppose that we never wrote this down anywhere, but when I became code > owner for the AA subsystem, we had an understanding that, because AA is so > pervasive and subtle, even as code owner I would never commit anything (aside > from reverts or similar) without pre-commit review. IMHO, this is the only > responsible way to handle this subsystem. Not all subsystems are so risky, so > there's appropriately a spectrum of development practices (some favoring > velocity over stability), but I still view this as being more democratic than > hierarchically authoritative. Fair enough - it sounded like the instigating incident in this case was maybe the other end of the spectrum - one developer telling another developer that they had to send more things for review when neither were code owner & they seem to disagree on that line. I'm not sure that's something we could/should codify. I think if there's ongoing disagreement about the suitable norms in part of the project it'd probably be useful for a code owner (or at least some precedence from a collection of developers in that space) to help codify practices/norms in that component. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77683/new/ https://reviews.llvm.org/D77683 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
