[LLVMdev] RFC: Code Ownership

[Chandler Carruth]

On Mon, Nov 12, 2012 at 7:35 AM, Duncan Sands <baldrick at free.fr> wrote:
> Hi,
>
>
> On 12/11/12 15:11, Meador Inge wrote:
>>
>> On 11/11/2012 11:58 PM, Chris Lattner wrote:
>>
>>>> Is there a particular sub-system size that makes sense to mark as owned?
>>>> I have been
>>>> reworking the library call simplification infrastructure recently and
>>>> will be happy
>>>> to sign up as an owner for that.
>>>
>>>
>>> I think that "directory level" is the right granularity.  If you're
>>> interested in signing
>>> up to maintain and review the whole instcombine library, that would be a
>>> great level.  To
>>> see what this entails, see:
>>> http://llvm.org/docs/DeveloperPolicy.html#code-owners
>>
>>
>> I am interested and comfortable with that.  I will wait a few days and
>> then
>> update CODE_OWNERS.TXT.
>
>
> while I think it's great that Meador is stepping forward to help out here,
> it
> does bring up the question of how expert you have to be to become a code
> owner.
> As far as I know (please correct me if I'm wrong) Meador isn't the number 1
> LLVM
> instcombine expert, but instead is someone who knows instcombine fairly well
> and is willing to spend time making sure instcombine is in good shape and
> stays
> that way.  My take is that you should be able to be a code owner without
> being
> the ultimate über hacker for that subsystem, since (1) the über hackers will
> have their eye on you and will let you know if you get it wrong; and (2)
> being
> willing counts for a lot.  More power to volunteers!  Hopefully if people
> talk
> with each other, help each other out, and maintain a friendly and
> cooperative
> atmosphere then all problems with code ownership will just come out in the
> wash.
>
> However another take on the whole code owner thing is that the code owner is
> the person who has the final say on what goes in and what doesn't, so it is
> important for the code owner to be the über guru (and having other people
> override their decisions would just undermine their authority and the whole
> code owner system).  Personally I don't really buy this, but I can
> understand
> the concern.

First: this is a relatively minor concern in the grander scheme of
things. I generally don't think it will come up often and will get
sorted out correctly if it does. However, when it does come up,
without being addressed, i suspect it will take a fair amount of time
and energy of the community to sort it all out. So maybe it's worth
addressing it now....


Personally, the reason for this concern is the conflation of several
independent roles into a single "code ownership" bucket:
- What the development policy says: ensuring each patch gets some review.
- What the release process follows: code owners get the final say in
whether a patch gets cherry picked into a release.
- What has happened in practice: code owners make decisions when there
are two viable choices with differing pros and cons, but no clear
consensus on which direction is preferred.

I think your first paragraph's process works very well for the first,
"ok" for the second, and not very well for the third. I think for the
third point (and to a lesser degree for the second), what you want is
*an* expert in the area, not necessarily *the* expert. Interestingly,
if we can address the third point, that also solves the only tricky
part of the second point: if there is a concern or lack of consensus
when bringing a patch into the release, the second point can magically
be promoted to the third.

I see two easy ways to avoid this issue:

1) Split off the third point from code ownership. Have some other
mechanism, which hopefully is not a single-point-of-failure (or
success... ;]).

2) Require code owners to be at least one of the "trusted maintainers"
of the area they own (to quote the dev policy).


I think either one of these would work quite well, and I have no real
preference for which we follow. #1 would be most consistent with the
wording of the documentation, and #2 would be most consistent with
existing precedent and activities of the community.